diff options
author | Kees Cook <keescook@chromium.org> | 2021-12-06 22:34:13 -0800 |
---|---|---|
committer | Mika Westerberg <mika.westerberg@linux.intel.com> | 2021-12-07 15:05:44 +0300 |
commit | 19813551701d004b517534888aa4e2a62ca4488e (patch) | |
tree | 326d4a3d9b24c995ed875c4e8e80fbe049b5e40b | |
parent | 0fcfb00b28c0b7884635dacf38e46d60bf3d4eb1 (diff) | |
download | linux-19813551701d004b517534888aa4e2a62ca4488e.tar.bz2 |
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: Kees Cook <keescook@chromium.org>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
-rw-r--r-- | drivers/thunderbolt/tb_msgs.h | 47 | ||||
-rw-r--r-- | drivers/thunderbolt/xdomain.c | 16 |
2 files changed, 36 insertions, 27 deletions
diff --git a/drivers/thunderbolt/tb_msgs.h b/drivers/thunderbolt/tb_msgs.h index bcabfcb2fd03..fe1afa44c56d 100644 --- a/drivers/thunderbolt/tb_msgs.h +++ b/drivers/thunderbolt/tb_msgs.h @@ -535,15 +535,25 @@ 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 { - struct tb_xdp_header hdr; - uuid_t src_uuid; - u32 src_route_hi; - u32 src_route_lo; + 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 { @@ -555,13 +565,18 @@ struct tb_xdp_properties { }; struct tb_xdp_properties_response { - struct tb_xdp_header hdr; - uuid_t src_uuid; - uuid_t dst_uuid; - u16 offset; - u16 data_length; - u32 generation; - u32 data[0]; + 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[]; + }; + }; }; /* @@ -580,7 +595,10 @@ struct tb_xdp_properties_changed { }; struct tb_xdp_properties_changed_response { - struct tb_xdp_header hdr; + union { + struct tb_xdp_error_response err; + struct tb_xdp_header hdr; + }; }; enum tb_xdp_error { @@ -591,9 +609,4 @@ enum tb_xdp_error { ERROR_NOT_READY, }; -struct tb_xdp_error_response { - struct tb_xdp_header hdr; - u32 error; -}; - #endif diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c index eff32499610f..01d6b724ca51 100644 --- a/drivers/thunderbolt/xdomain.c +++ b/drivers/thunderbolt/xdomain.c @@ -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 |