Hello!

On 5/21/23 23:22, Maxim Dounin wrote:
>> While the data being written was correctly written via memcpy(3),
>> you wouldn't be allowed to access it later as anything that is
>> not 'struct sockaddr'. For example, the following is a
>> strict-aliasing violation:
>>
>> struct s { int a; int b; };
>> struct t { int a; };
>> union u { struct s s; struct t t; };
>>
>> struct s x = {42, 42};
>> union u y;
>> int z;
>>
>> memcpy(&y.t, &x, sizeof(x)); // This is fine
>>
>> // We created an object of type 'struct t' in the union.
>> // Unions allow aliasing, so we're allowed to reinterpret
>> // that object as a 'struct s' via the other member.
>>
>> z = y.s.a; // This is fine.
>>
>> // But we're not allowed to reinterpret bytes that are
>> // officially uninitialized (even though we know they are
>> // initialized).
>>
>> z = y.s.b; // UB here.
>>
>> The reason for the UB is that the compiler is free to assume
>> that since you wrote to the struct t member, the write can't
>> possibly write to the second member of the struct (even if
>> the size passed to memcpy(3) is larger than that). In other
>> words, the compiler may assume that anything past
>> sizeof(struct t) is uninitialized.
>
> You haven't wrote to the struct t member, you wrote to the address
> using memcpy(). There is a difference, see C99 (or C11, whichever
> you prefer), 6.5 Expressions.

I assume you refer to this:

C11::6.5p6:
< The effective type of an object for an access to its stored
< value is the declared type of the object, if any.87) If a value
< is stored into an object having no declared type through an
< lvalue having a type that is not a character type, then the type
< of the lvalue becomes the effective type of the object for that
< access and for subsequent accesses that do not modify the stored
< value. If a value is copied into an object having no declared
< type using memcpy or memmove, or is copied as an array of
< character type, then the effective type of the modified object
< for that access and for subsequent accesses that do not modify
< the value is the effective type of the object from which the
< value is copied, if it has one. For all other accesses to an
< object having no declared type, the effective type of the object
< is simply the type of the lvalue used for the access.
<
[...]
<
< 87) Allocated objects have no declared type.


Let's break it into sentences:

< 87) Allocated objects have no declared type.

malloc(3)d memory has no declared type; all other memory has a
declared type.

< The effective type of an object for an access to its stored
< value is the declared type of the object, if any.89)

'y.t' has a declared type of 'struct t', so its effective type is
also 'struct t'.

< If a value is copied into an object having no declared
< type using memcpy or memmove, [...]

It doesn't apply, since 'y.t' has declared type. memcpy(3)
can't change the effective type in this case.


I think my example has UB (not 100% sure, though). However, I
notice now that it's slightly different from the one in nginx,
since nginx wasn't using a named variable (or at least it's not
obvious in ngx_quic_recvmsg(), since it's just receiving a
pointer), and it's instead probably using malloc(3)'d memory, so
the memory didn't have declared type. Only if a local temporary
variable had been used at some point upper in the call chain,
there could be UB. This is something that a static analyzer
will have a hard time checking (and a human reviewer too),
though.

https://software.codidact.com/posts/288138


>
>> Also, writing past an
>> object is very dubious, even via memcpy(3), even if you know
>> that the storage is there (thanks to the union). It's just
>> safer writing to the union itself, or to the field that has
>> the correct object type.
>
> And that's why the patch. While it is correct to write to the
> memory with any pointer,

Only if the memory is malloc(3)'d memory. But yes, probably
that's the case in patch. If local/global variables are
involved that's not allowed.

> using the union itself is more obvious
> and causes less confusion.

Yup; thanks for improving that.

Thanks,
Alex

>
> [...]
>

--
http://www.alejandro-colomar.es/
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel