[review] Avoid zero-length array at the end of struct link_map [BZ #25097]

["Carlos O'Donell (Code Review)" <gerrit at gnutoolchain-gerrit dot osci dot io>]

Carlos O'Donell has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488
......................................................................


Patch Set 1: Code-Review+2

(4 comments)

Looks good to me.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488/1/include/link.h 
File include/link.h:

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488/1/include/link.h@338 
PS1, Line 338: 
333 |    <ldsodefs.h>.  */
334 | struct auditstate
335 | {
336 |   uintptr_t cookie;
337 |   unsigned int bindflags;
338 > };
339 | 
340 | 
341 | #if __ELF_NATIVE_CLASS == 32
342 | # define symbind symbind32
343 | #elif __ELF_NATIVE_CLASS == 64

OK. Move definition out of the link_map struct.


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488/1/sysdeps/generic/ldsodefs.h 
File sysdeps/generic/ldsodefs.h:

https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488/1/sysdeps/generic/ldsodefs.h@387 
PS1, Line 387: 
382 |   /* Structure describing the dynamic linker itself.  */
383 |   EXTERN struct link_map _dl_rtld_map;
384 | #ifdef SHARED
385 |   /* Used to store the audit information for the link map of the
386 |      dynamic loader.  */
387 >   struct auditstate _dl_rtld_auditstate[DL_NNS];
388 | #endif
389 | 
390 | #if defined SHARED && defined _LIBC_REENTRANT \
391 |     && defined __rtld_lock_default_lock_recursive
392 |   EXTERN void (*_dl_rtld_lock_recursive) (void *);

OK. Renamed, but effectively the same.


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488/1/sysdeps/generic/ldsodefs.h@1184 
PS1, Line 1184: 
1179 | static inline struct auditstate *
1180 | link_map_audit_state (struct link_map *l, size_t index)
1181 | {
1182 |   if (l == &GL (dl_rtld_map))
1183 |     /* The auditstate array is stored separately.  */
1184 >     return &GL (dl_rtld_auditstate) [index];
1185 |   else
1186 |     {
1187 |       /* The auditstate array follows the link map in memory.  */
1188 |       struct auditstate *base = (struct auditstate *) (l + 1);
1189 |       return &base[index];

OK, for the normal layout of dl_rtld_map.


https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/488/1/sysdeps/generic/ldsodefs.h@1189 
PS1, Line 1189: 
1180 | link_map_audit_state (struct link_map *l, size_t index)
     | ...
1184 |     return &GL (dl_rtld_auditstate) [index];
1185 |   else
1186 |     {
1187 |       /* The auditstate array follows the link map in memory.  */
1188 |       struct auditstate *base = (struct auditstate *) (l + 1);
1189 >       return &base[index];
1190 |     }
1191 | }
1192 | #endif /* SHARED */
1193 | 
1194 | __END_DECLS

OK. We adjust the base pointer to point at the end of the link_map, then cast to an audit state structure. This is not an aliasing violation, the pointer is outside of the current structure. Once computed we take the array offset based on index and return that.



-- 
Gerrit-Project: glibc
Gerrit-Branch: master
Gerrit-Change-Id: Ic911100730f9124d4ea977ead8e13cee64b84d45
Gerrit-Change-Number: 488
Gerrit-PatchSet: 1
Gerrit-Owner: Florian Weimer <fweimer@redhat.com>
Gerrit-Reviewer: Carlos O'Donell <carlos@redhat.com>
Gerrit-Comment-Date: Sun, 03 Nov 2019 21:19:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment