HID: u2fzero: clarify error check and length calculations
authorAndrej Shadura <andrew.shadura@collabora.co.uk>
Tue, 19 Oct 2021 15:29:16 +0000 (17:29 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 18 Nov 2021 13:04:21 +0000 (14:04 +0100)
[ Upstream commit b7abf78b7a6c4a29a6e0ba0bb883fe44a2f3d693 ]

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.

To make the code easier to read, calculate the minimal packet length
once and separately, and assign it to a signed int variable to eliminate
unsigned math and the need for type casts. It then becomes immediately
obvious how the actual data length is calculated and why the return
value cannot be less than the minimal length.

Fixes: 22d65765f211 ("HID: u2fzero: ignore incomplete packets without data")
Fixes: 42337b9d4d95 ("HID: add driver for U2F Zero built-in LED and RNG")
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/hid/hid-u2fzero.c

index d70cd3d7f583b25f5736e464f7e8bac09deb302c..94f78ffb76d0434c815e0b8b9209c1607a3e053a 100644 (file)
@@ -191,6 +191,8 @@ static int u2fzero_rng_read(struct hwrng *rng, void *data,
        struct u2f_hid_msg resp;
        int ret;
        size_t actual_length;
+       /* valid packets must have a correct header */
+       int min_length = offsetof(struct u2f_hid_msg, init.data);
 
        if (!dev->present) {
                hid_dbg(dev->hdev, "device not present");
@@ -200,12 +202,12 @@ 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 < min_length)
                return 0;
 
        /* only take the minimum amount of data it is safe to take */
-       actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg,
-               init.data), U2F_HID_MSG_LEN(resp), max);
+       actual_length = min3((size_t)ret - min_length,
+               U2F_HID_MSG_LEN(resp), max);
 
        memcpy(data, resp.init.data, actual_length);