| Message ID | 20260701131336.3578279-1-richard.purdie@linuxfoundation.org |
|---|---|
| State | New |
| Headers | show |
| Series | [pseudo,1/7] pseudo.h: Avoid accessing unallocated memory | expand |
On 7/1/26 8:13 AM, Richard Purdie wrote: > We can call STARTSWITH in cases where the item being searched for is longer > than the string itself. Switch from memcmp to strncmp to avoid accessing > unassigned memory. > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > --- > pseudo.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/pseudo.h b/pseudo.h > index b6c13f2..1152c19 100644 > --- a/pseudo.h > +++ b/pseudo.h > @@ -99,7 +99,7 @@ extern char *pseudo_version; > #define PSEUDO_LIBDIR "lib" > #endif > > -#define STARTSWITH(x, y) (!memcmp((x), (y), sizeof(y) - 1)) > +#define STARTSWITH(x, y) (strncmp(y, x, strlen(y)) == 0) I've not looked at the users of STARTSWITH. If they are all strings this is fine. I believe memcmp was originally used because it could be items with nulls in it. The bug may be that 'sizeof' was not calculated properly (specifically checking both X and Y, only y.. which can lead that the issue.) > > #ifndef PSEUDO_LOCALSTATEDIR > #define PSEUDO_LOCALSTATEDIR "var/pseudo"
On Wed, 2026-07-01 at 11:40 -0500, Mark Hatle wrote: > On 7/1/26 8:13 AM, Richard Purdie wrote: > > We can call STARTSWITH in cases where the item being searched for is longer > > than the string itself. Switch from memcmp to strncmp to avoid accessing > > unassigned memory. > > > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > > --- > > pseudo.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/pseudo.h b/pseudo.h > > index b6c13f2..1152c19 100644 > > --- a/pseudo.h > > +++ b/pseudo.h > > @@ -99,7 +99,7 @@ extern char *pseudo_version; > > #define PSEUDO_LIBDIR "lib" > > #endif > > > > -#define STARTSWITH(x, y) (!memcmp((x), (y), sizeof(y) - 1)) > > +#define STARTSWITH(x, y) (strncmp(y, x, strlen(y)) == 0) > > I've not looked at the users of STARTSWITH. If they are all strings this is > fine. I believe memcmp was originally used because it could be items with nulls > in it. The bug may be that 'sizeof' was not calculated properly (specifically > checking both X and Y, only y.. which can lead that the issue.) It is only used by pseudo_util.c against envp: pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_LIBRARIES "=")) { pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_LIBRARIES "=")) { pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_PATH "=")) { pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_LIBRARIES "=")) continue; pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_PATH "=")) continue; i.e. always on strings. The bug is that y can be longer than x and the memcmp check doesn't check for that. I could have put more logic in there but we may as well just use strncmp. Cheers, Richard
On 7/1/26 11:47 AM, Richard Purdie via lists.yoctoproject.org wrote: > On Wed, 2026-07-01 at 11:40 -0500, Mark Hatle wrote: >> On 7/1/26 8:13 AM, Richard Purdie wrote: >>> We can call STARTSWITH in cases where the item being searched for is longer >>> than the string itself. Switch from memcmp to strncmp to avoid accessing >>> unassigned memory. >>> >>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> >>> --- >>> pseudo.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/pseudo.h b/pseudo.h >>> index b6c13f2..1152c19 100644 >>> --- a/pseudo.h >>> +++ b/pseudo.h >>> @@ -99,7 +99,7 @@ extern char *pseudo_version; >>> #define PSEUDO_LIBDIR "lib" >>> #endif >>> >>> -#define STARTSWITH(x, y) (!memcmp((x), (y), sizeof(y) - 1)) >>> +#define STARTSWITH(x, y) (strncmp(y, x, strlen(y)) == 0) >> >> I've not looked at the users of STARTSWITH. If they are all strings this is >> fine. I believe memcmp was originally used because it could be items with nulls >> in it. The bug may be that 'sizeof' was not calculated properly (specifically >> checking both X and Y, only y.. which can lead that the issue.) > > It is only used by pseudo_util.c against envp: > > pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_LIBRARIES "=")) { > pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_LIBRARIES "=")) { > pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_PATH "=")) { > pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_LIBRARIES "=")) continue; > pseudo_util.c: if (STARTSWITH(envp[i], PRELINK_PATH "=")) continue; > > i.e. always on strings. > > The bug is that y can be longer than x and the memcmp check doesn't > check for that. I could have put more logic in there but we may as well > just use strncmp. Agreed, if we're limiting to strings then better to use strncmp. --Mark > Cheers, > > Richard > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#4351): https://lists.yoctoproject.org/g/yocto-patches/message/4351 > Mute This Topic: https://lists.yoctoproject.org/mt/120064771/3616948 > Group Owner: yocto-patches+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org] > -=-=-=-=-=-=-=-=-=-=-=- > >
diff --git a/pseudo.h b/pseudo.h index b6c13f2..1152c19 100644 --- a/pseudo.h +++ b/pseudo.h @@ -99,7 +99,7 @@ extern char *pseudo_version; #define PSEUDO_LIBDIR "lib" #endif -#define STARTSWITH(x, y) (!memcmp((x), (y), sizeof(y) - 1)) +#define STARTSWITH(x, y) (strncmp(y, x, strlen(y)) == 0) #ifndef PSEUDO_LOCALSTATEDIR #define PSEUDO_LOCALSTATEDIR "var/pseudo"
We can call STARTSWITH in cases where the item being searched for is longer than the string itself. Switch from memcmp to strncmp to avoid accessing unassigned memory. Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- pseudo.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)