musl - RE: [PATCH] mman: correct length check in __shm

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <PH7PR11MB5795F548FFA02488A42934FCB3522@PH7PR11MB5795.namprd11.prod.outlook.com>
Date: Tue, 5 Nov 2024 02:03:21 +0000
From: "Zhao, Lihua (CN)" <Lihua.Zhao.CN@...driver.com>
To: Rich Felker <dalias@...c.org>
CC: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: RE: [PATCH] mman: correct length check in __shm_mapname

This issue is found by attached test case, it works well with glibc.

        sem_name[0] = '/';

        sem_name[NAME_MAX + 1] = '\0';

        memset(sem_name + 1, 'N', NAME_MAX);

        /* Create the semaphore */
        sem = sem_open(sem_name, O_CREAT, 0777, 1);

The above code will generate below string which has one '/' and 255 'N's:

"/NNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNNN"

When call __shm_mapname, it firstly try to skip the first '/' character, name point to the first 'N' character, the p will point to the EOS, so the p-name equal 255, the original code won't enter the ENAMETOOLONG branch. The name string should end with EOS, and all valid characters should be less than or equal to 254.

Thanks,
Lihua

-----Original Message-----
From: Rich Felker <dalias@...c.org> 
Sent: Tuesday, November 5, 2024 9:47 AM
To: Zhao, Lihua (CN) <Lihua.Zhao.CN@...driver.com>
Cc: musl@...ts.openwall.com
Subject: Re: [musl] [PATCH] mman: correct length check in __shm_mapname

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

On Tue, Nov 05, 2024 at 09:06:33AM +0800, lihua.zhao.cn@...driver.com wrote:
> From: Lihua Zhao <lihua.zhao.cn@...driver.com>
>
> changed the length check from `p-name > NAME_MAX` to `p-name >= 
> NAME_MAX` to correctly account for the null terminator.
>
> Signed-off-by: Lihua Zhao <lihua.zhao.cn@...driver.com>
> ---
>  src/mman/shm_open.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mman/shm_open.c b/src/mman/shm_open.c index 
> 79784bd3..2359f067 100644
> --- a/src/mman/shm_open.c
> +++ b/src/mman/shm_open.c
> @@ -15,7 +15,7 @@ char *__shm_mapname(const char *name, char *buf)
>               errno = EINVAL;
>               return 0;
>       }
> -     if (p-name > NAME_MAX) {
> +     if (p-name >= NAME_MAX) {
>               errno = ENAMETOOLONG;
>               return 0;
>       }
> --
> 2.43.0

This doesn't look correct. Can you explain what problem you think it's solving?

Rich

View attachment "test_sem_open.c" of type "text/plain" (2366 bytes)

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.