| Message ID | 20211018064800.15173-1-andrew.shadura@collabora.co.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [1/2] HID: u2fzero: explicitly check for errors | expand |
On Mon, Oct 18, 2021 at 08:47:59AM +0200, Andrej Shadura wrote: > The previous commit fixed handling of incomplete packets but broke error > handling: offsetof returns an unsigned value (size_t), but when compared > against the signed return value, the return value is interpreted as if > it were unsigned, so negative return values are never less than the > offset. > > Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without data") > Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG") Nit, you need a ' ' before the '(' character here, and you only need 12 digits of the sha1. It should look like: Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets without data") Also, how about a cc: stable for these as well? thanks, greg k-h
On 18/10/2021 08:56, Greg KH wrote: > On Mon, Oct 18, 2021 at 08:47:59AM +0200, Andrej Shadura wrote: >> The previous commit fixed handling of incomplete packets but broke error >> handling: offsetof returns an unsigned value (size_t), but when compared >> against the signed return value, the return value is interpreted as if >> it were unsigned, so negative return values are never less than the >> offset. >> >> Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without data") >> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG") > > Nit, you need a ' ' before the '(' character here, and you only need 12 > digits of the sha1. It should look like: > Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets without data") Thanks, is there a script or something to help generate those? (I’m surprised --fixup cannot generate them.) > Also, how about a cc: stable for these as well? Sure, will do.
On 18/10/2021 14:06, Andrej Shadura wrote: > On 18/10/2021 08:56, Greg KH wrote: >> On Mon, Oct 18, 2021 at 08:47:59AM +0200, Andrej Shadura wrote: >>> The previous commit fixed handling of incomplete packets but broke error >>> handling: offsetof returns an unsigned value (size_t), but when compared >>> against the signed return value, the return value is interpreted as if >>> it were unsigned, so negative return values are never less than the >>> offset. >>> >>> Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without >>> data") >>> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and >>> RNG") >> >> Nit, you need a ' ' before the '(' character here, and you only need 12 >> digits of the sha1. It should look like: >> Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets >> without data") > > Thanks, is there a script or something to help generate those? (I’m > surprised --fixup cannot generate them.) Replying to myself, apparently the answer to this is now in the "submitting patches" document.
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c index d70cd3d7f583..5145d758bea0 100644 --- a/drivers/hid/hid-u2fzero.c +++ b/drivers/hid/hid-u2fzero.c @@ -200,7 +200,7 @@ static int u2fzero_rng_read(struct hwrng *rng, void *data, ret = u2fzero_recv(dev, &req, &resp); /* ignore errors or packets without data */ - if (ret < offsetof(struct u2f_hid_msg, init.data)) + if (ret < 0 || ret < offsetof(struct u2f_hid_msg, init.data)) return 0; /* only take the minimum amount of data it is safe to take */
The previous commit fixed handling of incomplete packets but broke error handling: offsetof returns an unsigned value (size_t), but when compared against the signed return value, the return value is interpreted as if it were unsigned, so negative return values are never less than the offset. Fixes: 22d65765f211c("HID: u2fzero: ignore incomplete packets without data") Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG") Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk> --- drivers/hid/hid-u2fzero.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)