Commit 19813551 authored by Kees Cook's avatar Kees Cook Committed by Mika Westerberg
Browse files

thunderbolt: xdomain: Avoid potential stack OOB read 



tb_xdp_properties_changed_request() was calling tb_xdp_handle_error() with
a struct tb_xdp_properties_changed_response on the stack, which does not
have the "error" field present when cast to struct tb_xdp_error_response.
This was detected when building with -Warray-bounds:

drivers/thunderbolt/xdomain.c: In function 'tb_xdomain_properties_changed':
drivers/thunderbolt/xdomain.c:226:22: error: array subscript 'const struct tb_xdp_error_response[0]' is partly outside array bounds of 'struct tb_xdp_properties_changed_response[1]' [-Werror=array-bounds]
  226 |         switch (error->error) {
      |                 ~~~~~^~~~~~~
drivers/thunderbolt/xdomain.c:448:51: note: while referencing 'res'
  448 |         struct tb_xdp_properties_changed_response res;
      |                                                   ^~~

Add union containing struct tb_xdp_error_response to structures passed
to tb_xdp_handle_error(), so that the "error" field will be present.

Signed-off-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatarMika Westerberg <mika.westerberg@linux.intel.com>
parent 0fcfb00b

drivers/thunderbolt/tb_msgs.h

+30 −17
Original line number Diff line number Diff line
@@ -535,16 +535,26 @@ struct tb_xdp_header { 
	u32 type;

};



struct tb_xdp_error_response {

	struct tb_xdp_header hdr;

	u32 error;

};



struct tb_xdp_uuid {

	struct tb_xdp_header hdr;

};



struct tb_xdp_uuid_response {

	union {

		struct tb_xdp_error_response err;

		struct {

			struct tb_xdp_header hdr;

			uuid_t src_uuid;

			u32 src_route_hi;

			u32 src_route_lo;

		};

	};

};



struct tb_xdp_properties {

	struct tb_xdp_header hdr;
@@ -555,13 +565,18 @@ struct tb_xdp_properties { 
};



struct tb_xdp_properties_response {

	union {

		struct tb_xdp_error_response err;

		struct {

			struct tb_xdp_header hdr;

			uuid_t src_uuid;

			uuid_t dst_uuid;

			u16 offset;

			u16 data_length;

			u32 generation;

	u32 data[0];

			u32 data[];

		};

	};

};



/*
@@ -580,8 +595,11 @@ struct tb_xdp_properties_changed { 
};



struct tb_xdp_properties_changed_response {

	union {

		struct tb_xdp_error_response err;

		struct tb_xdp_header hdr;

	};

};



enum tb_xdp_error {

	ERROR_SUCCESS,
@@ -591,9 +609,4 @@ enum tb_xdp_error { 
	ERROR_NOT_READY,

};



struct tb_xdp_error_response {

	struct tb_xdp_header hdr;

	u32 error;

};



#endif

drivers/thunderbolt/xdomain.c

+6 −10
Original line number Diff line number Diff line
@@ -214,16 +214,12 @@ static inline void tb_xdp_fill_header(struct tb_xdp_header *hdr, u64 route, 
	memcpy(&hdr->uuid, &tb_xdp_uuid, sizeof(tb_xdp_uuid));

}



static int tb_xdp_handle_error(const struct tb_xdp_header *hdr)

static int tb_xdp_handle_error(const struct tb_xdp_error_response *res)

{

	const struct tb_xdp_error_response *error;



	if (hdr->type != ERROR_RESPONSE)

	if (res->hdr.type != ERROR_RESPONSE)

		return 0;



	error = (const struct tb_xdp_error_response *)hdr;



	switch (error->error) {

	switch (res->error) {

	case ERROR_UNKNOWN_PACKET:

	case ERROR_UNKNOWN_DOMAIN:

		return -EIO;
@@ -257,7 +253,7 @@ static int tb_xdp_uuid_request(struct tb_ctl *ctl, u64 route, int retry, 
	if (ret)

		return ret;



	ret = tb_xdp_handle_error(&res.hdr);

	ret = tb_xdp_handle_error(&res.err);

	if (ret)

		return ret;
@@ -329,7 +325,7 @@ static int tb_xdp_properties_request(struct tb_ctl *ctl, u64 route, 
		if (ret)

			goto err;



		ret = tb_xdp_handle_error(&res->hdr);

		ret = tb_xdp_handle_error(&res->err);

		if (ret)

			goto err;
@@ -462,7 +458,7 @@ static int tb_xdp_properties_changed_request(struct tb_ctl *ctl, u64 route, 
	if (ret)

		return ret;



	return tb_xdp_handle_error(&res.hdr);

	return tb_xdp_handle_error(&res.err);

}



static int