install-xattr: avoid accessing empty storage#3
Closed
thesamesam wants to merge 6 commits intogentoo:masterfrom
thesamesam:install-xattr-ubsan
Closed
install-xattr: avoid accessing empty storage#3thesamesam wants to merge 6 commits intogentoo:masterfrom thesamesam:install-xattr-ubsan
thesamesam wants to merge 6 commits intogentoo:masterfrom
thesamesam:install-xattr-ubsan
Conversation
Member
Author
SoapGentoo
approved these changes
Jan 6, 2023
mgorny
reviewed
Jan 6, 2023
SoapGentoo
reviewed
Jan 9, 2023
SoapGentoo
suggested changes
Jan 9, 2023
Needed to correctly run tests with Clang, as Clang doesn't create executable stacks by default. Signed-off-by: Sam James <sam@gentoo.org>
It's hard to see why something failed otherwise, as we only have the exit code. Signed-off-by: Sam James <sam@gentoo.org>
UBSAN reports:
```
install-xattr.c:124:16: runtime error: load of address 0x55555556d440 with insufficient space for an object of type 'char'
0x55555556d440: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 61 00 00 00
^
#0 0x555555557a27 in copyxattr /home/sam/git/elfix/misc/install-xattr/install-xattr.c:124
#1 0x555555556a4d in main /home/sam/git/elfix/misc/install-xattr/install-xattr.c:410
#2 0x7ffff77c864f (/usr/lib64/libc.so.6+0x2364f)
#3 0x7ffff77c8708 in __libc_start_main (/usr/lib64/libc.so.6+0x23708)
#4 0x555555557114 in _start (/home/sam/git/elfix/misc/install-xattr/install-xattr+0x3114)
```
Triggered with:
```
mkdir /tmp/a
touch /tmp/foo
./install-xattr -c /tmp/foo /tmp/foo2 /tmp/a
```
I don't see this with Clang or < GCC 12, but I do with GCC 13 (13.0.0_pre20230101 p5);
I suspect it's because of object-size improvements.
Signed-off-by: Sam James <sam@gentoo.org>
There's another with strdup/malloc but it gets a bit messier
to fix so let's leave that for now (this is mostly about correctness
anyway, as the runtime of install-xattr is very small):
```
Direct leak of 4097 byte(s) in 1 object(s) allocated from:
#0 0x7f4a2c22e257 in __interceptor_malloc /usr/src/debug/sys-devel/gcc-13.0.0_pre20230101/gcc-13-20230101/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7f4a2c1d2b40 in __interceptor_realpath /usr/src/debug/sys-devel/gcc-13.0.0_pre20230101/gcc-13-20230101/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:3904
#2 0x55da3adf5629 in realpath /usr/include/bits/stdlib.h:42
#3 0x55da3adf5629 in main /home/sam/git/elfix/misc/install-xattr/install-xattr.c:252
```
Signed-off-by: Sam James <sam@gentoo.org>
Obsolete. Signed-off-by: Sam James <sam@gentoo.org>
Signed-off-by: Sam James <sam@gentoo.org>
SoapGentoo
approved these changes
Jan 9, 2023
gentoo-bot
pushed a commit
that referenced
this pull request
Jan 10, 2023
UBSAN reports:
```
install-xattr.c:124:16: runtime error: load of address 0x55555556d440 with insufficient space for an object of type 'char'
0x55555556d440: note: pointer points here
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 61 00 00 00
^
#0 0x555555557a27 in copyxattr /home/sam/git/elfix/misc/install-xattr/install-xattr.c:124
#1 0x555555556a4d in main /home/sam/git/elfix/misc/install-xattr/install-xattr.c:410
#2 0x7ffff77c864f (/usr/lib64/libc.so.6+0x2364f)
#3 0x7ffff77c8708 in __libc_start_main (/usr/lib64/libc.so.6+0x23708)
#4 0x555555557114 in _start (/home/sam/git/elfix/misc/install-xattr/install-xattr+0x3114)
```
Triggered with:
```
mkdir /tmp/a
touch /tmp/foo
./install-xattr -c /tmp/foo /tmp/foo2 /tmp/a
```
I don't see this with Clang or < GCC 12, but I do with GCC 13 (13.0.0_pre20230101 p5);
I suspect it's because of object-size improvements.
Signed-off-by: Sam James <sam@gentoo.org>
gentoo-bot
pushed a commit
that referenced
this pull request
Jan 10, 2023
There's another with strdup/malloc but it gets a bit messier
to fix so let's leave that for now (this is mostly about correctness
anyway, as the runtime of install-xattr is very small):
```
Direct leak of 4097 byte(s) in 1 object(s) allocated from:
#0 0x7f4a2c22e257 in __interceptor_malloc /usr/src/debug/sys-devel/gcc-13.0.0_pre20230101/gcc-13-20230101/libsanitizer/asan/asan_malloc_linux.cpp:69
#1 0x7f4a2c1d2b40 in __interceptor_realpath /usr/src/debug/sys-devel/gcc-13.0.0_pre20230101/gcc-13-20230101/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:3904
#2 0x55da3adf5629 in realpath /usr/include/bits/stdlib.h:42
#3 0x55da3adf5629 in main /home/sam/git/elfix/misc/install-xattr/install-xattr.c:252
```
Signed-off-by: Sam James <sam@gentoo.org>
gentoo-bot
pushed a commit
to gentoo/gentoo
that referenced
this pull request
Jan 11, 2023
Bug: gentoo/elfix#3 Signed-off-by: Sam James <sam@gentoo.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
UBSAN reports:
Triggered with:
I don't see this with Clang or < GCC 12, but I do with GCC 13 (13.0.0_pre20230101 p5); I suspect it's because of object-size improvements.
Signed-off-by: Sam James sam@gentoo.org