systemd: do not use alloca() function in case of uclibc
cover systemd: do not use alloca() function in case of uclibc
The following changes since commit af347d3298e15552d502d5b2ce497bbda9705bc7:
binutils: Fix building nativesdk binutils with gcc 4.9 (2014-05-29 13:42:13 +0100)
are available in the git repository at:
git://git.openembedded.org/openembedded-core-contrib ChenQi/daisy-systemd-alloca
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=ChenQi/daisy-systemd-alloca
Chen Qi (1):
systemd: do not use alloca() function in case of uclibc
.../0001-journal-file.c-do-not-use-alloca.patch | 54 ++++++++++++++++++++ meta/recipes-core/systemd/systemd_211.bb | 1 + 2 files changed, 55 insertions(+) create mode 100644 meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch
Signature · 1 line
1.7.9.5
PATCH 1/1 systemd: do not use alloca() function in case of uclibc
The alloca() function allocates space in the stack frame of the caller, so using alloca(new_size - old_size) would possibly crash the stack, causing a segment fault error.
This patch fixes the above problem by avoiding using this function in journal-file.c.
[YOCTO #6201]
--- .../0001-journal-file.c-do-not-use-alloca.patch | 54 ++++++++++++++++++++ meta/recipes-core/systemd/systemd_211.bb | 1 + 2 files changed, 55 insertions(+) create mode 100644 meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch
meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch new file +54 -0
@@ -0,0 +1,54 @@ +Upstream-Status: Inappropriate [oe specific] + +journal-file.c: do not use alloca + +Using alloca(new_size - old_size) would possibly cause a segment fault +error. Note that the alloca function allocates space in the stack frame +of the caller. So it's possible that the allocated size exceeds the stack +size limit. + +There's actually no need to allocate stack here, we only need to write some +bytes to the block to make sure the needed block space is available, just like +eglibc does in posix_fallocate. + +Signed-off-by: Chen Qi <[email protected]> + +--- + src/journal/journal-file.c | 21 +++++++++------------ + 1 file changed, 9 insertions(+), 12 deletions(-) + +diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c +index cef8bba..e364298 100644 +--- a/src/journal/journal-file.c ++++ b/src/journal/journal-file.c +@@ -371,18 +371,15 @@ static int journal_file_allocate(JournalFile *f, uint64_t offset, uint64_t size) + if (r != 0) + return -r; + #else +- /* Use good old method to write zeros into the journal file +- perhaps very inefficient yet working. */ +- if(new_size > old_size) { +- char *buf = alloca(new_size - old_size); +- off_t oldpos = lseek(f->fd, 0, SEEK_CUR); +- bzero(buf, new_size - old_size); +- lseek(f->fd, old_size, SEEK_SET); +- r = write(f->fd, buf, new_size - old_size); +- lseek(f->fd, oldpos, SEEK_SET); +- } +- if (r < 0) +- return -errno; ++ /* Write something every 512 bytes to make sure the block is allocated */ ++ uint64_t len = new_size - old_size; ++ uint64_t offset = old_size; ++ for (offset += (len-1) % 512; len > 0; offset += 512) { ++ len -= 512; ++ if (pwrite(f->fd, "", 1, offset) != 1) ++ return -errno; ++ } ++ + #endif /* HAVE_POSIX_FALLOCATE */ + + if (fstat(f->fd, &f->last_stat) < 0) +-- +1.7.9.5 +
meta/recipes-core/systemd/systemd_211.bb +1 -0
@@ -32,6 +32,7 @@ SRC_URI = "git://anongit.freedesktop.org/systemd/systemd;branch=master;protocol= file://uclibc-sysinfo_h.patch \ file://uclibc-get-physmem.patch \ file://sd-bus-don-t-use-assert_return-to-check-for-disconne.patch \ + file://0001-journal-file.c-do-not-use-alloca.patch \ \ file://touchscreen.rules \ file://00-create-volatile.conf \
Signature · 1 line
1.7.9.5
Quoted 22 lines · Chen Qi
The
alloca()function allocates space in the stack frame of the caller, so usingalloca(new_size - old_size)would possibly crash the stack, causing a segment fault error.This patch fixes the above problem by avoiding using this function in
journal-file.c.[YOCTO #6201]
Signed-off-by Chen Qi <[email protected]>--- .../0001-journal-file.c-do-not-use-alloca.patch | 54 ++++++++++++++++++++ meta/recipes-core/systemd/systemd_211.bb | 1 + 2 files changed, 55 insertions(+) create mode 100644 meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patchmeta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch new file +0
-0
meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch line 1
Quoted 1 line
meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch new file +1
-0@@ -0,0 +1,54 @@ +Upstream-Status: Inappropriate [oe specific]
From the description, this sounds like an allocation error which can happen *anywhere* and is a problem that should be addressed upstream.
This Upstream-Status field is therefore completely bogus. Its not inappropriate or oe specific. If you still believe it is, I'd like to hear more explanation.
The abuses of this field are starting to really annoy me since this keeps happening.
Cheers,
Richard
Quoted 31 lines · Richard Purdie
Quoted 23 lines · Chen Qi
The
alloca()function allocates space in the stack frame of the caller, so usingalloca(new_size - old_size)would possibly crash the stack, causing a segment fault error.This patch fixes the above problem by avoiding using this function in
journal-file.c.[YOCTO #6201]
Signed-off-by Chen Qi <[email protected]>--- .../0001-journal-file.c-do-not-use-alloca.patch | 54 ++++++++++++++++++++ meta/recipes-core/systemd/systemd_211.bb | 1 + 2 files changed, 55 insertions(+)create mode 100644
meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patchmeta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch new file +1
-0@@ -0,0 +1,54 @@ +Upstream-Status: Inappropriate [oe specific]Quoted 1 line
From the description, this sounds like an allocation error which can
happen *anywhere* and is a problem that should be addressed upstream.
This Upstream-Status field is therefore completely bogus. Its not inappropriate or oe specific. If you still believe it is, I'd like to hear more explanation.
Quoted 11 lines
The abuses of this field are starting to really annoy me since this keeps happening.
Cheers,
Richard
Hi Richard,
The use of alloca() was introduced by an oe-specific patch from Khem Raj.
The patch is meta/recipes-core/systemd/systemd/systemd-pam-fix-fallocate.patch. The upstream status of the above patch is as following.
Upstream-Status: Denied [no desire for uclibc support]
That's why I use 'Inappropriate [oe specific]' in the Upstream-Status field of my patch.
And I just realized I forgot to also patch the journald-kmsg.c file. I'll send out a V2.
Best Regards,
Chen Qi
Quoted 52 lines · ChenQi
Quoted 35 lines · Richard Purdie
Quoted 27 lines · Chen Qi
The
alloca()function allocates space in the stack frame of the caller, so usingalloca(new_size - old_size)would possibly crash the stack, causing a segment fault error.This patch fixes the above problem by avoiding using this function in
journal-file.c.[YOCTO #6201]
Signed-off-by Chen Qi <[email protected]>--- .../0001-journal-file.c-do-not-use-alloca.patch | 54 ++++++++++++++++++++ meta/recipes-core/systemd/systemd_211.bb | 1 + 2 files changed, 55 insertions(+)create mode 100644
meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patchmeta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch +0
-0new file mode 100644
index 0000000..a638d58
---/dev/null
+++b/meta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patchmeta/recipes-core/systemd/systemd/0001-journal-file.c-do-not-use-alloca.patch +1
-0@@ -0,0 +1,54 @@ +Upstream-Status: Inappropriate [oe specific]Quoted 1 line
From the description, this sounds like an allocation error which can
happen *anywhere* and is a problem that should be addressed upstream.
This Upstream-Status field is therefore completely bogus. Its not inappropriate or oe specific. If you still believe it is, I'd like to hear more explanation.
Quoted 11 lines
The abuses of this field are starting to really annoy me since this keeps happening.
Cheers,
Richard
Hi Richard,
The use of
alloca()was introduced by an oe-specific patch from Khem Raj.
Quoted 8 lines
The patch is
meta/recipes-core/systemd/systemd/systemd-pam-fix-fallocate.patch. The upstream status of the above patch is as following.
Upstream-Status: Denied [no desire for uclibc support]That's why I use 'Inappropriate [oe specific]' in the Upstream-Status field of my patch.
And I just realized I forgot to also patch the
journald-kmsg.cfile. I'll send out a V2.
Sorry for the confusion.
I just checked and journald-kmsg.c doesn't have the same problem, as it's only allocating a small size of space.
char *buf = alloca(sizeof(uint64_t));
So I think that's it.
I will also send out a patch for master branch.
Best Regards,
Chen Qi
Quoted 2 lines
Best Regards,
Chen Qi
Quoted 7 lines · ChenQi
The use of
alloca()was introduced by an oe-specific patch from Khem Raj.The patch is
meta/recipes-core/systemd/systemd/systemd-pam-fix-fallocate.patch. The upstream status of the above patch is as following.
Upstream-Status: Denied [no desire for uclibc support]
Quoted 5 lines
That's why I use 'Inappropriate [oe specific]' in the Upstream-Status field of my patch.
And I just realized I forgot to also patch the
journald-kmsg.cfile. I'll send out a V2.
Please just update that patch then rather than patching code that we've ready patched. Is this issue uclibc specific?
At the very least information like this needs to be put into the commit message/patch header.
Cheers,
Richard
Richard Purdie <richard.purdie@...> writes:
Hi,
Quoted 2 lines
Please just update that patch then rather than patching code that we've ready patched. Is this issue uclibc specific?
the usage of alloca is due me and killing a lot of malloc's in the hot path of log handling. I am late to these patches but my two cents would be that if you use uclibc you really want to have alloca for the speed.
holger