diff mbox series

[v2] mmc-utils: fix potential overflow

Message ID 20230926131128.3771508-1-giulio.benetti@benettiengineering.com (mailing list archive)
State New, archived
Headers show
Series [v2] mmc-utils: fix potential overflow | expand

Commit Message

Giulio Benetti Sept. 26, 2023, 1:11 p.m. UTC
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/a53922c5db3e605a5e81e53c034f45017ebb7db7/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/a53922c5db3e605a5e81e53c034f45017ebb7db7/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.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
V1->V2:
* corrected commit log
---
 mmc_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Avri Altman Sept. 26, 2023, 1:15 p.m. UTC | #1
> 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.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

> ---
> V1->V2:
> * corrected commit log
> ---
>  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
Ulf Hansson Sept. 27, 2023, 11:05 a.m. UTC | #2
On Tue, 26 Sept 2023 at 15:11, Giulio Benetti
<giulio.benetti@benettiengineering.com> wrote:
>
> 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/a53922c5db3e605a5e81e53c034f45017ebb7db7/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/a53922c5db3e605a5e81e53c034f45017ebb7db7/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.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

Applied to git.kernel.org/pub/scm/utils/mmc/mmc-utils.git master, thanks!

Kind regards
Uffe


> ---
> V1->V2:
> * corrected commit log
> ---
>  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
>
diff mbox series

Patch

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;										\
 	})