On Thu, Jun 21, 2018 at 3:00 AM, Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> wrote: > On Wed, Jun 20, 2018 at 11:28:40AM -0700, Kees Cook wrote: >> In the quest to remove all stack VLA usage from the kernel[1], this >> uses the maximum buffer size and adds a sanity check. >> >> [1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@xxxxxxxxxxxxxx >> >> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> >> --- >> drivers/usb/typec/tps6598x.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c >> index 4b4c8d271b27..396193f85e6d 100644 >> --- a/drivers/usb/typec/tps6598x.c >> +++ b/drivers/usb/typec/tps6598x.c >> @@ -81,12 +81,17 @@ struct tps6598x { >> struct typec_capability typec_cap; >> }; >> >> +#define TPS_MAX_LEN sizeof(u64) > > That is not big enough. The registers of this chip can be as big as 64 > bytes. The identity register alone is 25 bytes, so the above would > make the driver fail quite fast. Can you set the maximum to 64? > > #define TPS_MAX_LEN 64 Oops! Yes, thanks, I missed this usage: struct tps6598x_rx_identity_reg id; ... ret = tps6598x_block_read(tps, TPS_REG_RX_IDENTITY_SOP, &id, sizeof(id)); struct tps6598x_rx_identity_reg { u8 status; /* 0 1 */ struct usb_pd_identity identity; /* 1 12 */ u32 vdo[3]; /* 13 12 */ /* size: 25, cachelines: 1, members: 3 */ /* last cacheline: 25 bytes */ }; But that's still only 25 bytes. Where is the 64? I see these: return tps6598x_block_read(tps, reg, val, sizeof(u16)); return tps6598x_block_read(tps, reg, val, sizeof(u32)); return tps6598x_block_read(tps, reg, val, sizeof(u64)); ret = tps6598x_block_read(tps, TPS_REG_RX_IDENTITY_SOP, &id, sizeof(id)); ret = tps6598x_block_read(tps, TPS_REG_DATA1, out_data, out_len); ret = tps6598x_block_read(tps, TPS_REG_DATA1, &val, sizeof(u8)); 1, 2, 4, 8, 25, and "out_len" in tps6598x_exec_cmd(), which is always zero: ret = tps6598x_exec_cmd(tps, cmd, 0, NULL, 0, NULL); ret = tps6598x_exec_cmd(tps, cmd, 0, NULL, 0, NULL); I clearly need a v2 of this patch, but I just want to make sure I get the right value. :) Thanks for the review! -Kees -- Kees Cook Pixel Security -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
