Re: [PATCH] mmc-utils: fix potential overflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Avri,

On 26/09/23 11:54, Avri Altman wrote:
With fortify enabled gcc throws following error:
                  from mmc_cmds.c:20:
In function ‘read’,
     inlined from ‘do_rpmb_write_key’ at mmc_cmds.c:2233:8:
/home/giuliobenetti/br_reproduce/a53922c5db3e605a5e81e53c034f45017e
bb7db7/output/host/mipsel-buildroot-linux-
gnu/sysroot/usr/include/bits/unistd.h:38:10: error: ‘__read_alias’ writing
228 or more bytes into a region of size 32 overflows the destination [-
Werror=stringop-overflow=]
    38 |   return __glibc_fortify (read, __nbytes, sizeof (char),
       |          ^~~~~~~~~~~~~~~
mmc_cmds.c: In function ‘do_rpmb_write_key’:
mmc_cmds.c:2087:19: note: destination object ‘key_mac’ of size 32
  2087 |         u_int8_t  key_mac[32];
       |                   ^~~~~~~
/home/giuliobenetti/br_reproduce/a53922c5db3e605a5e81e53c034f45017e
bb7db7/output/host/mipsel-buildroot-linux-
gnu/sysroot/usr/include/bits/unistd.h:26:16: note: in a call to function
‘__read_alias’ declared with attribute ‘access (write_only, 2, 3)’
    26 | extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
       |                ^~~~~~~~~~

read() could potentially return more than nbyte so let's check for ret < nbyte.
No it can't because that would mean buffer overflow.
See https://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html :
" this number shall never be greater than nbyte."

So this seems like a bogus warning to me.

Yes, you're right, and after doing some test this error appear with _FORTIFY_SOURCE=3 but it doesn't with _FORTIFY_SOURCE=2 or 1.

So yes. I'm going to reword like:
```
Building with _FORTIFY_SOURCE=3 results in:
                 from mmc_cmds.c:20:
In function ‘read’,
    inlined from ‘do_rpmb_write_key’ at mmc_cmds.c:2233:8:
/home/giuliobenetti/br_reproduce/a53922c5db3e605a5e81e53c034f45017e
bb7db7/output/host/mipsel-buildroot-linux-
gnu/sysroot/usr/include/bits/unistd.h:38:10: error: ‘__read_alias’ writing
228 or more bytes into a region of size 32 overflows the destination [-
Werror=stringop-overflow=]
   38 |   return __glibc_fortify (read, __nbytes, sizeof (char),
      |          ^~~~~~~~~~~~~~~
mmc_cmds.c: In function ‘do_rpmb_write_key’:
mmc_cmds.c:2087:19: note: destination object ‘key_mac’ of size 32
 2087 |         u_int8_t  key_mac[32];
      |                   ^~~~~~~
/home/giuliobenetti/br_reproduce/a53922c5db3e605a5e81e53c034f45017e
bb7db7/output/host/mipsel-buildroot-linux-
gnu/sysroot/usr/include/bits/unistd.h:26:16: note: in a call to function
‘__read_alias’ declared with attribute ‘access (write_only, 2, 3)’
   26 | extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
      |                ^~~~~~~~~~

To work around this let's check if the return of read() is lower than
the nbyte requested instead of not equal.
```

Does it look good for you?

Best regards
--
Giulio Benetti
CEO&CTO@Benetti Engineering sas

Thanks,
Avri

Signed-off-by: Giulio Benetti <giulio.benetti@xxxxxxxxxxxxxxxxxxxxxx>
---
  mmc_cmds.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mmc_cmds.c b/mmc_cmds.c
index 10d063d..ae7b876 100644
--- a/mmc_cmds.c
+++ b/mmc_cmds.c
@@ -2065,7 +2065,7 @@ int do_sanitize(int nargs, char **argv)
                         }                                                                               \
                         else if (r > 0)                                                 \
                                 ret += r;                                                       \
-               } while (r != 0 && (size_t)ret != nbyte);       \
+               } while (r != 0 && (size_t)ret < nbyte);        \
                                                                                                         \
                 ret;                                                                            \
         })
--
2.34.1





[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux