Bug 284563 - Holes in struct rtld_utrace
Summary: Holes in struct rtld_utrace
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: misc (show other bugs)
Version: 14.2-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Konstantin Belousov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-02-04 13:49 UTC by Paul Floyd
Modified: 2025-02-14 20:21 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2025-02-04 13:49:16 UTC
The definition of struct rtld_utrace is

struct utrace_rtld {
	char sig[RTLD_UTRACE_SIG_SZ];
	int event;
        /* 4 byte hole */
	void *handle;
	void *mapbase;			/* Used for 'parent' and 'init/fini' */
	size_t mapsize;
	int refcnt;			/* Used for 'mode' */
        /* 4 byte hole */
	char name[MAXPATHLEN];
};

And the interface for utrace is

335	AUE_NULL	STD|CAPENABLED {
		int utrace(
		    _In_reads_bytes_(len) const void *addr,
		    size_t len
		);
	}

I don't know what other uses of utrace exise, but rtld ld_utrace_log does not memset utrace_rtld  to 0 before filling the fields, leaving the holes uninitialized.

This poses a problem for Valgrind memcheck testing the syscall parameters. At present it just uses the pointer and length to indicate memory that will be read in the syscall (abd should be initialized). I can fix this in Valgrind by using struct utrace_rtld.

I think that a better fix would be on the FreeBSD side by putting the 'event' and 'refcnt' fields together. Would that be possible without breaking backwards compatibility?
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2025-02-04 22:37:16 UTC
The name[] member does not need alignment.
Regardless, would clearing the whole structure memory helps Valgrind?

https://reviews.freebsd.org/D48854
Comment 2 Paul Floyd 2025-02-05 06:16:59 UTC
(In reply to Konstantin Belousov from comment #1)

You're right, I was too quick looking at the struct. Here is the pahole output

struct utrace_rtld {
        char                       sig[4];               /*     0     4 */
        int                        event;                /*     4     4 */
        void *                     handle;               /*     8     8 */
        void *                     mapbase;              /*    16     8 */
        size_t                     mapsize;              /*    24     8 */
        int                        refcnt;               /*    32     4 */
        char                       name[1024];           /*    36  1024 */

        /* size: 1064, cachelines: 17, members: 7 */
        /* padding: 4 */
        /* last cacheline: 40 bytes */
};

so there is just the 4 bytes of padding at the end that is uninitialized and was triggering errors. The i386 ld.so is OK since everything is a multiple of 4.

I've already fixed this in Valgrind. I don't know whether MSAN has a similar issue.

Zeroing the entire 1064 bytes would also fix the problem for sure.
Comment 3 commit-hook freebsd_committer freebsd_triage 2025-02-05 10:34:33 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e917958c36670131ab42e8f2c849b708a3216e37

commit e917958c36670131ab42e8f2c849b708a3216e37
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2025-02-04 22:33:11 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2025-02-05 10:33:29 +0000

    rtld: clear any holes in the struct utrace_rtld passed to kernel logger

    This should avoid an (almost) false positive from Valgrind, by filling
    the padding on LP64.

    PR:     284563
    Reported by:    Paul Floyd <pjfloyd@wanadoo.fr>
    Reviewed by:    emaste
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D48854

 libexec/rtld-elf/rtld.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Comment 4 commit-hook freebsd_committer freebsd_triage 2025-02-08 00:38:41 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=068de5be49a296491027df01c0bb1731c5b2ca56

commit 068de5be49a296491027df01c0bb1731c5b2ca56
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2025-02-04 22:33:11 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2025-02-08 00:26:55 +0000

    rtld: clear any holes in the struct utrace_rtld passed to kernel logger

    PR:     284563

    (cherry picked from commit e917958c36670131ab42e8f2c849b708a3216e37)

 libexec/rtld-elf/rtld.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)