[musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[puwenxu]

[-- Attachment #1.1: Type: text/plain, Size: 843 bytes --]

Dear maintainer,
       I'm using musl 1.2.3.0 now. When I was running some test case codes for musl, I found there may be a problem on modification of 2b2c8aafce9d80f9d58652643538f4d58e82b856.
       As you can see in the picture, this modification assign buf to orig. Then, the orig will be assigned to buf again. If the original value of buf is NULL, the latter value of the buf will be NULL. However, assign out to buf will fail if buf is NULL.
       [cid:image001.png@01D8EC6C.12AE8540]
       I have written a simple file to verification my opinion. The test code and output is shown in the following picture. I think it may be better to add a check for buf in this situation.
       [cid:image002.png@01D8EC6C.12AE8540]
[cid:image003.png@01D8EC6C.12AE8540]
Looking forward to your reply.



Wish Pu
       OS Kernel Lab


[-- Attachment #1.2: Type: text/html, Size: 4674 bytes --]

[-- Attachment #2: image001.png --]
[-- Type: image/png, Size: 100094 bytes --]

[-- Attachment #3: image002.png --]
[-- Type: image/png, Size: 101290 bytes --]

[-- Attachment #4: image003.png --]
[-- Type: image/png, Size: 4073 bytes --]

Re: [musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[Sam James]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]



> On 30 Oct 2022, at 06:29, puwenxu <puwenxu1@huawei.com> wrote:
> 
> Dear maintainer,
>        I’m using musl 1.2.3.0 now. When I was running some test case codes for musl, I found there may be a problem on modification of 2b2c8aafce9d80f9d58652643538f4d58e82b856.

FWIW, for these reports, it's much more accessible if you link to regions of a file or quote them rather than including images.

Images aren't easy for someone using a screen-reader or for someone using a client which doesn't support it.

Best,
sam


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 358 bytes --]

Re: [musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[Rich Felker]

On Sun, Oct 30, 2022 at 06:29:54AM +0000, puwenxu wrote:
> Dear maintainer,
>        I'm using musl 1.2.3.0 now. When I was running some test case
> codes for musl, I found there may be a problem on modification of
> 2b2c8aafce9d80f9d58652643538f4d58e82b856.
>        As you can see in the picture, this modification assign buf
> to orig. Then, the orig will be assigned to buf again. If the
> original value of buf is NULL, the latter value of the buf will be
> NULL. However, assign out to buf will fail if buf is NULL.

Rather than pointing at what you think is wrong in the source change,
can you demonstrate a minimal example of calling code that was correct
and worked before the change, but fails after the change?

As best I can tell, your concern is about the case where you pass a
null pointer as buf when using one of the msgctl commands that
requires a pointer to a buffer. This is undefined.

>        I have written a simple file to verification my opinion. The
> test code and output is shown in the following picture. I think it
> may be better to add a check for buf in this situation.

This isn't an example. An example would be a minimal program that
calls msgctl in a valid (i.e. no undefined behavior) way and
malfunctions as a result of the change.

Rich

Re: [musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[Markus Wichmann]

On Sun, Oct 30, 2022 at 06:29:54AM +0000, puwenxu wrote:
> Dear maintainer, I'm using musl 1.2.3.0 now. When I was running some
> test case codes for musl, I found there may be a problem on
> modification of 2b2c8aafce9d80f9d58652643538f4d58e82b856.  As you can
> see in the picture, this modification assign buf to orig. Then, the
> orig will be assigned to buf again. If the original value of buf is
> NULL, the latter value of the buf will be NULL. However, assign out to
> buf will fail if buf is NULL.  [cid:image001.png@01D8EC6C.12AE8540] I
> have written a simple file to verification my opinion. The test code
> and output is shown in the following picture. I think it may be better
> to add a check for buf in this situation.
> [cid:image002.png@01D8EC6C.12AE8540]
> [cid:image003.png@01D8EC6C.12AE8540] Looking forward to your reply.
>
>
>
> Wish Pu OS Kernel Lab
>

Hi,

as the others already said, please avoid images if text would also do.

I do also note that it appears you Huawei people have a bee in your
bonnet about NULL pointer checks. musl does not plaster over bad
behavior. In the cases where a NULL pointer argument would be
dereferenced, it was undefined behavior to give a NULL pointer argument
in the first place. I do agree that programs need to be prevented from
dereferencing NULL pointers, but the place to do so is after creating
the pointers and before handing it to an API that does not accept NULL
pointers. And yes, it does suck that C is incapable of expressing the
idea of a non-nullable pointer natively, but C is the language we're
stuck with.

Your test program only shows that by default, dereferencing the NULL
pointer on Linux crashes. This is well-known, though not behavior you
can rely upon since it can be overridden if you know how. In any case,
having a pointer which ought to point at an object point to nothing is a
logic error that needs to be rectified in the calling code. Just check
your malloc() returns, it isn't that difficult!

Ciao,
Markus

[musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[puwenxu]

[-- Attachment #1.1: Type: text/plain, Size: 505 bytes --]

Dear maintainer,
       I'm sorry for my last question firstly.
       I'm not sure whether I have right to show you full test codes because the discipline of my company is strict at it. The error occurs when the test case is running the following code.
err = msgctl(msqid, IPC_STAT, NULL);
       As you can see, the parameter buf is NULL. In this situation, the orig in msgctl will be NULL, and then cause the problem I described last time.
    [cid:image003.png@01D8ED82.534EBC70]


Wish Pu


[-- Attachment #1.2: Type: text/html, Size: 3394 bytes --]

[-- Attachment #2: image003.png --]
[-- Type: image/png, Size: 133604 bytes --]

Re: [musl] Question on 2b2c8aafce9d80f9d58652643538f4d58e82b856

[Rich Felker]

On Mon, Oct 31, 2022 at 03:41:43PM +0000, puwenxu wrote:
> Dear maintainer,
>        I'm sorry for my last question firstly.

>        I'm not sure whether I have right to show you full test codes
> because the discipline of my company is strict at it. The error
> occurs when the test case is running the following code.
> err = msgctl(msqid, IPC_STAT, NULL);

This is an erroneous test case. The contract of msgctl with the
IPC_STAT command is that the buf pointer must point to an object of
type struct msqid_ds. NULL does not point to such an object.

Rich