| Message ID | 20250416-nolibc-ubsan-v1-3-c4704bb23da7@weissschuh.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: fix some undefined behaviour and enable UBSAN | expand |
On Wed, Apr 16, 2025 at 08:40:18PM +0200, Thomas Weißschuh wrote: > As byte buffer is overlaid with a 'struct dirent64'. > it has to satisfy the structs alignment requirements. > > Fixes: 665fa8dea90d ("tools/nolibc: add support for directory access") > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Oh good catch! I already faced crashes in other programs due to AVX memcpy when doing similar casts without thinking about alignment. > @@ -58,6 +58,7 @@ int closedir(DIR *dirp) > static __attribute__((unused)) > int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result) > { > + __attribute__((aligned(__alignof__(struct linux_dirent64)))) > char buf[sizeof(struct linux_dirent64) + NAME_MAX + 1]; I tend to find it more readable when the attribute is on the same line as the type on variables declaration, often at the end of the line, to keep declarations alignes, even if that makes longer lines. If alignment concerns come back often, we could maybe have __nolibc_align(<align>) and maybe even __nolibc_align_as(<type>) to slightly shorten the lines. Just an idea. In any case: Acked-by: Willy Tarreau <w@1wt.eu> Willy
On 2025-04-19 11:11:40+0200, Willy Tarreau wrote: > On Wed, Apr 16, 2025 at 08:40:18PM +0200, Thomas Weißschuh wrote: > > As byte buffer is overlaid with a 'struct dirent64'. > > it has to satisfy the structs alignment requirements. > > > > Fixes: 665fa8dea90d ("tools/nolibc: add support for directory access") > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > Oh good catch! I already faced crashes in other programs due to AVX > memcpy when doing similar casts without thinking about alignment. > > > @@ -58,6 +58,7 @@ int closedir(DIR *dirp) > > static __attribute__((unused)) > > int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result) > > { > > + __attribute__((aligned(__alignof__(struct linux_dirent64)))) > > char buf[sizeof(struct linux_dirent64) + NAME_MAX + 1]; > > I tend to find it more readable when the attribute is on the same line as > the type on variables declaration, often at the end of the line, to keep > declarations alignes, even if that makes longer lines. If alignment concerns > come back often, we could maybe have __nolibc_align(<align>) and maybe even > __nolibc_align_as(<type>) to slightly shorten the lines. Just an idea. Sounds good, the attribute soup is annoying. > In any case: Acked-by: Willy Tarreau <w@1wt.eu> Thanks! Thomas
diff --git a/tools/include/nolibc/dirent.h b/tools/include/nolibc/dirent.h index c5c30d0dd6806b1bec2fa8120a3df29aaa201393..cd0ddff86c360b14913a809c8696d89d8a356e9e 100644 --- a/tools/include/nolibc/dirent.h +++ b/tools/include/nolibc/dirent.h @@ -58,6 +58,7 @@ int closedir(DIR *dirp) static __attribute__((unused)) int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result) { + __attribute__((aligned(__alignof__(struct linux_dirent64)))) char buf[sizeof(struct linux_dirent64) + NAME_MAX + 1]; struct linux_dirent64 *ldir = (void *)buf; intptr_t i = (intptr_t)dirp;
As byte buffer is overlaid with a 'struct dirent64'. it has to satisfy the structs alignment requirements. Fixes: 665fa8dea90d ("tools/nolibc: add support for directory access") Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- tools/include/nolibc/dirent.h | 1 + 1 file changed, 1 insertion(+)