* [musl] Dynamic linker segfault @ 2022-01-04 21:27 Nihal Jere 2022-01-04 21:42 ` Rich Felker 0 siblings, 1 reply; 9+ messages in thread From: Nihal Jere @ 2022-01-04 21:27 UTC (permalink / raw) To: musl Hi, ldd from master segfaults when run on [1] and [2]. It happens on this[3] line. It seems to happen due to the intersection of a few factors: 1. The segment at the lowest address is read-only. 2. A segment on the the same page is read/write. 3. The read/write segment has memsz > filesz. This results in a segfault, as it tries to memset[3] on the mmap created here[4], which has the same protection as the segment at the lowest address (i.e. read-only). As far as I can see, the options are to either: a. detect this and throw an error. b. 'or' together the protection flags of all the segments on the page. I'm not sure what the right behavior is, but I don't think segfaulting is right, and I'm sure there are people here what's correct. Best, Nihal [1] https://github.com/golang/go/blob/master/src/debug/elf/testdata/go-relocation-test-gcc930-ranges-no-rela-x86-64 [2] https://github.com/golang/go/blob/master/src/debug/elf/testdata/go-relocation-test-gcc930-ranges-with-rela-x86-64 [3] https://git.musl-libc.org/cgit/musl/tree/ldso/dynlink.c#n783 [4] https://git.musl-libc.org/cgit/musl/tree/ldso/dynlink.c#n742 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-04 21:27 [musl] Dynamic linker segfault Nihal Jere @ 2022-01-04 21:42 ` Rich Felker 2022-01-05 8:56 ` Florian Weimer 0 siblings, 1 reply; 9+ messages in thread From: Rich Felker @ 2022-01-04 21:42 UTC (permalink / raw) To: Nihal Jere; +Cc: musl On Tue, Jan 04, 2022 at 03:27:10PM -0600, Nihal Jere wrote: > Hi, > > ldd from master segfaults when run on [1] and [2]. It happens on > this[3] line. It seems to happen due to the intersection of a few > factors: > > 1. The segment at the lowest address is read-only. > 2. A segment on the the same page is read/write. > 3. The read/write segment has memsz > filesz. This is a malformed program file not compatible with the machine page size (4k). Arguably it should be detected as p_align < PAGESIZE -- in a sense, p_align for LOAD segments is the maximum supported page size for the program file, and machines not capable of providing a page size that small can't map/run it. In theory the loader could allow this if all the differences between segments satisfy the right congruences and have matching permissions where the maps would overlap, but I'm not sure that's useful. What tooling generated this file? > This results in a segfault, as it tries to memset[3] on the mmap > created here[4], which has the same protection as the segment at > the lowest address (i.e. read-only). > > As far as I can see, the options are to either: > > a. detect this and throw an error. > b. 'or' together the protection flags of all the segments on the page. > > I'm not sure what the right behavior is, but I don't think segfaulting > is right, and I'm sure there are people here what's correct. There is a huge range of "syntactically valid but not correct" things ELF can represent, and musl has never attempted to catch or diagnose all such errors. It might be desirable to increase the level to which we catch common ones, but proceeding on this probably depends on how this file came to be. If it was made by intentionally passing bad options to the linker or with a custom linker that's not honoring the semantic requirements for an executable for the platform, I think there's a lot of lower-hanging fruit if we wanted to start diagnosing this sort of thing. Of course diagnosing p_align < PAGESIZE as an error might make sense in general. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-04 21:42 ` Rich Felker @ 2022-01-05 8:56 ` Florian Weimer 2022-01-05 13:49 ` Rich Felker 0 siblings, 1 reply; 9+ messages in thread From: Florian Weimer @ 2022-01-05 8:56 UTC (permalink / raw) To: Rich Felker; +Cc: Nihal Jere, musl * Rich Felker: > This is a malformed program file not compatible with the machine page > size (4k). Arguably it should be detected as p_align < PAGESIZE -- in > a sense, p_align for LOAD segments is the maximum supported page size > for the program file, and machines not capable of providing a page > size that small can't map/run it. In theory the loader could allow > this if all the differences between segments satisfy the right > congruences and have matching permissions where the maps would > overlap, but I'm not sure that's useful. We've been looking at this on the glibc side recently. The use case is supporting large data alignments (greater than the kernel page size) while not pessimizing multi-page-size targets such as POWER and AArch64. With a p_align < PAGESIZE check, binaries need to specify p_align as the largest support kernel page size (64K on those targets). On a 64K page kernel, this alignment happens naturally, but on a 4K (or 16K) kernel, the loader needs to carve out a larger memory area and align it manually to achieve 64K alignment (because that's what p_align says). So far this didn't happen because we only checked p_align against the kernel size, we did not actually use it for aligning the mappings … Thanks, Florian ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-05 8:56 ` Florian Weimer @ 2022-01-05 13:49 ` Rich Felker 2022-01-05 14:00 ` Florian Weimer 0 siblings, 1 reply; 9+ messages in thread From: Rich Felker @ 2022-01-05 13:49 UTC (permalink / raw) To: Florian Weimer; +Cc: Nihal Jere, musl On Wed, Jan 05, 2022 at 09:56:25AM +0100, Florian Weimer wrote: > * Rich Felker: > > > This is a malformed program file not compatible with the machine page > > size (4k). Arguably it should be detected as p_align < PAGESIZE -- in > > a sense, p_align for LOAD segments is the maximum supported page size > > for the program file, and machines not capable of providing a page > > size that small can't map/run it. In theory the loader could allow > > this if all the differences between segments satisfy the right > > congruences and have matching permissions where the maps would > > overlap, but I'm not sure that's useful. > > We've been looking at this on the glibc side recently. The use case is > supporting large data alignments (greater than the kernel page size) > while not pessimizing multi-page-size targets such as POWER and AArch64. I'm not clear how it pessimizes these targets (beyond what's fundamentally necessary) unless you're artificially aligning segment contents on disk to a large alignment boundary to prevent over-mapping (undermining separate-code for example). And if you're doing that, you need the full alignment anyway to support machines with larger hardware pagesize. Otherwise you'd get back the overmapping (and unwanted perission exposure). The only other thing I can think of is pessimizing ASLR by requiring more alignment (throwing away a few bits of position entropy). In any case, do you know if this test file is somehow related to that work, or is it just a guess? It doesn't seem to be related to me since it's essentially a "pageless" mapping setup. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-05 13:49 ` Rich Felker @ 2022-01-05 14:00 ` Florian Weimer 2022-01-05 15:00 ` Rich Felker 0 siblings, 1 reply; 9+ messages in thread From: Florian Weimer @ 2022-01-05 14:00 UTC (permalink / raw) To: Rich Felker; +Cc: Nihal Jere, musl * Rich Felker: > On Wed, Jan 05, 2022 at 09:56:25AM +0100, Florian Weimer wrote: >> * Rich Felker: >> >> > This is a malformed program file not compatible with the machine page >> > size (4k). Arguably it should be detected as p_align < PAGESIZE -- in >> > a sense, p_align for LOAD segments is the maximum supported page size >> > for the program file, and machines not capable of providing a page >> > size that small can't map/run it. In theory the loader could allow >> > this if all the differences between segments satisfy the right >> > congruences and have matching permissions where the maps would >> > overlap, but I'm not sure that's useful. >> >> We've been looking at this on the glibc side recently. The use case is >> supporting large data alignments (greater than the kernel page size) >> while not pessimizing multi-page-size targets such as POWER and AArch64. > > I'm not clear how it pessimizes these targets (beyond what's > fundamentally necessary) unless you're artificially aligning segment > contents on disk to a large alignment boundary to prevent over-mapping > (undermining separate-code for example). And if you're doing that, you > need the full alignment anyway to support machines with larger > hardware pagesize. Otherwise you'd get back the overmapping (and > unwanted perission exposure). Hmm. Maybe I should rephrase: With a p_align < PAGESIZE check in place, portable binaries need to use the value 65536. When running with page size 4096, the loader cannot know whether p_align was set to this value merely to satisfy the p_align < PAGESIZE check, or because there is actually some section alignment that requires 65536 byte alignment. There is no kernel interface to request 65536 byte alignment, so the loader has to do extra work to satisfy this request. And in the first case (no actual 65536 byte alignment requirement), that work is unnecessary. > In any case, do you know if this test file is somehow related to that > work, or is it just a guess? It doesn't seem to be related to me since > it's essentially a "pageless" mapping setup. The glibc test seems to be just buggy: First we verify p_align against the page size, then we use that p_align value to check the alignment of the PT_LOAD segment (mainly file offset congruency). The p_align check against the page size looks completely optional to me if we check file offset congruency directly against the run-time page size. The ELF specification explicitly describes the p_align values 0 and 1 as valid, indicating no alignment constraint. So a p_align < PAGESIZE check is buggy in that regard as well. This also conflicts with your interpretation as p_align as the maximum supported page size. Thanks, Florian ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-05 14:00 ` Florian Weimer @ 2022-01-05 15:00 ` Rich Felker 2022-01-07 13:58 ` Florian Weimer 0 siblings, 1 reply; 9+ messages in thread From: Rich Felker @ 2022-01-05 15:00 UTC (permalink / raw) To: Florian Weimer; +Cc: Nihal Jere, musl On Wed, Jan 05, 2022 at 03:00:53PM +0100, Florian Weimer wrote: > * Rich Felker: > > > On Wed, Jan 05, 2022 at 09:56:25AM +0100, Florian Weimer wrote: > >> * Rich Felker: > >> > >> > This is a malformed program file not compatible with the machine page > >> > size (4k). Arguably it should be detected as p_align < PAGESIZE -- in > >> > a sense, p_align for LOAD segments is the maximum supported page size > >> > for the program file, and machines not capable of providing a page > >> > size that small can't map/run it. In theory the loader could allow > >> > this if all the differences between segments satisfy the right > >> > congruences and have matching permissions where the maps would > >> > overlap, but I'm not sure that's useful. > >> > >> We've been looking at this on the glibc side recently. The use case is > >> supporting large data alignments (greater than the kernel page size) > >> while not pessimizing multi-page-size targets such as POWER and AArch64. > > > > I'm not clear how it pessimizes these targets (beyond what's > > fundamentally necessary) unless you're artificially aligning segment > > contents on disk to a large alignment boundary to prevent over-mapping > > (undermining separate-code for example). And if you're doing that, you > > need the full alignment anyway to support machines with larger > > hardware pagesize. Otherwise you'd get back the overmapping (and > > unwanted perission exposure). > > Hmm. Maybe I should rephrase: > > With a p_align < PAGESIZE check in place, portable binaries need to use > the value 65536. When running with page size 4096, the loader cannot > know whether p_align was set to this value merely to satisfy the p_align > < PAGESIZE check, or because there is actually some section alignment > that requires 65536 byte alignment. There is no kernel interface to > request 65536 byte alignment, so the loader has to do extra work to > satisfy this request. And in the first case (no actual 65536 byte > alignment requirement), that work is unnecessary. Unfortunately it's impossible to distinguish between such an alignment requirement and __attribute__((__aligned__(65536))) appearing in section contents that went into the segment, so disregarding it is a bug (one musl also has) I think. > > In any case, do you know if this test file is somehow related to that > > work, or is it just a guess? It doesn't seem to be related to me since > > it's essentially a "pageless" mapping setup. > > The glibc test seems to be just buggy: First we verify p_align against > the page size, then we use that p_align value to check the alignment of > the PT_LOAD segment (mainly file offset congruency). The p_align check > against the page size looks completely optional to me if we check file > offset congruency directly against the run-time page size. > > The ELF specification explicitly describes the p_align values 0 and 1 as > valid, indicating no alignment constraint. So a p_align < PAGESIZE > check is buggy in that regard as well. This also conflicts with your > interpretation as p_align as the maximum supported page size. The ELF specification describes syntax not semantic requirements on the platform to support anything the syntax can represent. The semantics of the above can't be honored (without memcpy instead of mmap, which is really something you don't want to support) if they produce congruences incompatible with the layout, and they can't be honored at all if they're incompatible with the permissions requirements. Even when they don't conflict with the permissions requirements, they may expose unintended mapped data (which for users wanting separate-code is problematic, since it introduces gadgets). So I'm not convinced it should be supported even when it "can". Note also that segments (PT_LOAD) are not the only type of program headers, and p_align of 0 or 1 may make more sense for other types. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-05 15:00 ` Rich Felker @ 2022-01-07 13:58 ` Florian Weimer 2022-01-07 17:35 ` Rich Felker 0 siblings, 1 reply; 9+ messages in thread From: Florian Weimer @ 2022-01-07 13:58 UTC (permalink / raw) To: Rich Felker; +Cc: Nihal Jere, musl * Rich Felker: >> With a p_align < PAGESIZE check in place, portable binaries need to use >> the value 65536. When running with page size 4096, the loader cannot >> know whether p_align was set to this value merely to satisfy the p_align >> < PAGESIZE check, or because there is actually some section alignment >> that requires 65536 byte alignment. There is no kernel interface to >> request 65536 byte alignment, so the loader has to do extra work to >> satisfy this request. And in the first case (no actual 65536 byte >> alignment requirement), that work is unnecessary. > > Unfortunately it's impossible to distinguish between such an alignment > requirement and __attribute__((__aligned__(65536))) appearing in > section contents that went into the segment, so disregarding it is a > bug (one musl also has) I think. Agreed. >> > In any case, do you know if this test file is somehow related to that >> > work, or is it just a guess? It doesn't seem to be related to me since >> > it's essentially a "pageless" mapping setup. >> >> The glibc test seems to be just buggy: First we verify p_align against >> the page size, then we use that p_align value to check the alignment of >> the PT_LOAD segment (mainly file offset congruency). The p_align check >> against the page size looks completely optional to me if we check file >> offset congruency directly against the run-time page size. >> >> The ELF specification explicitly describes the p_align values 0 and 1 as >> valid, indicating no alignment constraint. So a p_align < PAGESIZE >> check is buggy in that regard as well. This also conflicts with your >> interpretation as p_align as the maximum supported page size. > > The ELF specification describes syntax not semantic requirements on > the platform to support anything the syntax can represent. That's not entirely accurate, there are exceptions. p_align seems to be one such exception: p_align As ``Program Loading'' describes in this chapter of the processor supplement, loadable process segments must have congruent values for p_vaddr and p_offset, modulo the page size. This member gives the value to which the segments are aligned in memory and in the file. Values 0 and 1 mean no alignment is required. Otherwise, p_align should be a positive, integral power of 2, and p_vaddr should equal p_offset, modulo p_align. Most processor supplements do not discuss p_align unfortunately? > The semantics of the above can't be honored (without memcpy instead of > mmap, which is really something you don't want to support) if they > produce congruences incompatible with the layout, and they can't be > honored at all if they're incompatible with the permissions > requirements. Even when they don't conflict with the permissions > requirements, they may expose unintended mapped data (which for users > wanting separate-code is problematic, since it introduces gadgets). So > I'm not convinced it should be supported even when it "can". The congruency compatibility is a property of the file layout, not the p_align value. If the file layout is incompatible, changing p_align doesn't make the object loadable. As far as I can tell, p_align is not at all useful, except for (e.g.) __attribute__((__aligned__(65536))) for global. But even that wasn't implemented widely. Thanks, Florian ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-07 13:58 ` Florian Weimer @ 2022-01-07 17:35 ` Rich Felker 2022-01-10 13:30 ` Florian Weimer 0 siblings, 1 reply; 9+ messages in thread From: Rich Felker @ 2022-01-07 17:35 UTC (permalink / raw) To: Florian Weimer; +Cc: Nihal Jere, musl On Fri, Jan 07, 2022 at 02:58:54PM +0100, Florian Weimer wrote: > * Rich Felker: > > >> With a p_align < PAGESIZE check in place, portable binaries need to use > >> the value 65536. When running with page size 4096, the loader cannot > >> know whether p_align was set to this value merely to satisfy the p_align > >> < PAGESIZE check, or because there is actually some section alignment > >> that requires 65536 byte alignment. There is no kernel interface to > >> request 65536 byte alignment, so the loader has to do extra work to > >> satisfy this request. And in the first case (no actual 65536 byte > >> alignment requirement), that work is unnecessary. > > > > Unfortunately it's impossible to distinguish between such an alignment > > requirement and __attribute__((__aligned__(65536))) appearing in > > section contents that went into the segment, so disregarding it is a > > bug (one musl also has) I think. > > Agreed. > > >> > In any case, do you know if this test file is somehow related to that > >> > work, or is it just a guess? It doesn't seem to be related to me since > >> > it's essentially a "pageless" mapping setup. > >> > >> The glibc test seems to be just buggy: First we verify p_align against > >> the page size, then we use that p_align value to check the alignment of > >> the PT_LOAD segment (mainly file offset congruency). The p_align check > >> against the page size looks completely optional to me if we check file > >> offset congruency directly against the run-time page size. > >> > >> The ELF specification explicitly describes the p_align values 0 and 1 as > >> valid, indicating no alignment constraint. So a p_align < PAGESIZE > >> check is buggy in that regard as well. This also conflicts with your > >> interpretation as p_align as the maximum supported page size. > > > > The ELF specification describes syntax not semantic requirements on > > the platform to support anything the syntax can represent. > > That's not entirely accurate, there are exceptions. p_align seems to be > one such exception: > > p_align > As ``Program Loading'' describes in this chapter of the processor > supplement, loadable process segments must have congruent values for > p_vaddr and p_offset, modulo the page size. This member gives the > value to which the segments are aligned in memory and in the > file. Values 0 and 1 mean no alignment is required. Otherwise, > p_align should be a positive, integral power of 2, and p_vaddr > should equal p_offset, modulo p_align. > > Most processor supplements do not discuss p_align unfortunately? > > > The semantics of the above can't be honored (without memcpy instead of > > mmap, which is really something you don't want to support) if they > > produce congruences incompatible with the layout, and they can't be > > honored at all if they're incompatible with the permissions > > requirements. Even when they don't conflict with the permissions > > requirements, they may expose unintended mapped data (which for users > > wanting separate-code is problematic, since it introduces gadgets). So > > I'm not convinced it should be supported even when it "can". > > The congruency compatibility is a property of the file layout, not the > p_align value. If the file layout is incompatible, changing p_align > doesn't make the object loadable. > > As far as I can tell, p_align is not at all useful, except for (e.g.) > __attribute__((__aligned__(65536))) for global. But even that wasn't > implemented widely. By my understanding, p_align implies an understanding by the creator of the program that the load segment may be "over-mapped" (extra file contents visible and possibly executable) up to that alignment boundary due to page granularity. This isn't really a correctness contract but a security/hardening contract for folks who consider anti-ROP measures a boundary that should be enforced. Rich ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [musl] Dynamic linker segfault 2022-01-07 17:35 ` Rich Felker @ 2022-01-10 13:30 ` Florian Weimer 0 siblings, 0 replies; 9+ messages in thread From: Florian Weimer @ 2022-01-10 13:30 UTC (permalink / raw) To: Rich Felker; +Cc: Nihal Jere, musl * Rich Felker: > By my understanding, p_align implies an understanding by the creator > of the program that the load segment may be "over-mapped" (extra file > contents visible and possibly executable) up to that alignment > boundary due to page granularity. This isn't really a correctness > contract but a security/hardening contract for folks who consider > anti-ROP measures a boundary that should be enforced. Not sure if I follow. Do you mean p_align is purely for documenting layout decisions by the link editor? I suspect that the necessary overlap would be visible by other means, too. Thanks, Florian ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-10 13:30 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-04 21:27 [musl] Dynamic linker segfault Nihal Jere 2022-01-04 21:42 ` Rich Felker 2022-01-05 8:56 ` Florian Weimer 2022-01-05 13:49 ` Rich Felker 2022-01-05 14:00 ` Florian Weimer 2022-01-05 15:00 ` Rich Felker 2022-01-07 13:58 ` Florian Weimer 2022-01-07 17:35 ` Rich Felker 2022-01-10 13:30 ` Florian Weimer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).