* [musl] [PATCH] dynlink: fix get_lfs64() with posix_fallocate64 @ 2024-05-10 16:01 Florian Ziesche 2024-05-23 20:28 ` Rich Felker 0 siblings, 1 reply; 4+ messages in thread From: Florian Ziesche @ 2024-05-10 16:01 UTC (permalink / raw) To: musl Hi, this patch increases the buffer size by one in get_lfs64() so that it works with posix_fallocate64. "posix_fallocate64" is 17 characters long, so 16 is one too short. Simplified example: before: https://compiler-explorer.com/z/4qcPhcaWr after: https://compiler-explorer.com/z/scGvhddKW --- diff --git a/ldso/dynlink.c b/ldso/dynlink.c index 42687da2..8707ae1c 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -363,7 +363,7 @@ static struct symdef get_lfs64(const char *name) "stat\0statfs\0statvfs\0tmpfile\0truncate\0versionsort\0" "__fxstat\0__fxstatat\0__lxstat\0__xstat\0"; size_t l; - char buf[16]; + char buf[17]; for (l=0; name[l]; l++) { if (l >= sizeof buf) goto nomatch; buf[l] = name[l]; ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] [PATCH] dynlink: fix get_lfs64() with posix_fallocate64 2024-05-10 16:01 [musl] [PATCH] dynlink: fix get_lfs64() with posix_fallocate64 Florian Ziesche @ 2024-05-23 20:28 ` Rich Felker 2024-06-22 1:06 ` Rich Felker 0 siblings, 1 reply; 4+ messages in thread From: Rich Felker @ 2024-05-23 20:28 UTC (permalink / raw) To: Florian Ziesche; +Cc: musl On Fri, May 10, 2024 at 06:01:58PM +0200, Florian Ziesche wrote: > Hi, > > this patch increases the buffer size by one in get_lfs64() so that it > works with posix_fallocate64. > "posix_fallocate64" is 17 characters long, so 16 is one too short. > > Simplified example: > before: https://compiler-explorer.com/z/4qcPhcaWr > after: https://compiler-explorer.com/z/scGvhddKW > > --- > diff --git a/ldso/dynlink.c b/ldso/dynlink.c > index 42687da2..8707ae1c 100644 > --- a/ldso/dynlink.c > +++ b/ldso/dynlink.c > @@ -363,7 +363,7 @@ static struct symdef get_lfs64(const char *name) > "stat\0statfs\0statvfs\0tmpfile\0truncate\0versionsort\0" > "__fxstat\0__fxstatat\0__lxstat\0__xstat\0"; > size_t l; > - char buf[16]; > + char buf[17]; > for (l=0; name[l]; l++) { > if (l >= sizeof buf) goto nomatch; > buf[l] = name[l]; Thanks for catching this. I questioned whether the 17 is sufficient, but indeed the buffer is never nul terminated except when removing the 64, so it should be ok. I think I'll apply your patch as a direct fix, but the whole copying operation is unnecessary and I'll probably remove it. It works just as well to strncmp and pass p instead of buf to find_sym, so that a mutable copy of the name to lookup is never needed. Rich ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] [PATCH] dynlink: fix get_lfs64() with posix_fallocate64 2024-05-23 20:28 ` Rich Felker @ 2024-06-22 1:06 ` Rich Felker 2024-06-22 18:37 ` Florian Ziesche 0 siblings, 1 reply; 4+ messages in thread From: Rich Felker @ 2024-06-22 1:06 UTC (permalink / raw) To: Florian Ziesche; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 1532 bytes --] On Thu, May 23, 2024 at 04:28:03PM -0400, Rich Felker wrote: > On Fri, May 10, 2024 at 06:01:58PM +0200, Florian Ziesche wrote: > > Hi, > > > > this patch increases the buffer size by one in get_lfs64() so that it > > works with posix_fallocate64. > > "posix_fallocate64" is 17 characters long, so 16 is one too short. > > > > Simplified example: > > before: https://compiler-explorer.com/z/4qcPhcaWr > > after: https://compiler-explorer.com/z/scGvhddKW > > > > --- > > diff --git a/ldso/dynlink.c b/ldso/dynlink.c > > index 42687da2..8707ae1c 100644 > > --- a/ldso/dynlink.c > > +++ b/ldso/dynlink.c > > @@ -363,7 +363,7 @@ static struct symdef get_lfs64(const char *name) > > "stat\0statfs\0statvfs\0tmpfile\0truncate\0versionsort\0" > > "__fxstat\0__fxstatat\0__lxstat\0__xstat\0"; > > size_t l; > > - char buf[16]; > > + char buf[17]; > > for (l=0; name[l]; l++) { > > if (l >= sizeof buf) goto nomatch; > > buf[l] = name[l]; > > Thanks for catching this. I questioned whether the 17 is sufficient, > but indeed the buffer is never nul terminated except when removing the > 64, so it should be ok. > > I think I'll apply your patch as a direct fix, but the whole copying > operation is unnecessary and I'll probably remove it. It works just as > well to strncmp and pass p instead of buf to find_sym, so that a > mutable copy of the name to lookup is never needed. Applied. Does the attached look ok to get rid of the copy entirely? It passed basic smoke testing here. [-- Attachment #2: get_lfs64.diff --] [-- Type: text/plain, Size: 919 bytes --] diff --git a/ldso/dynlink.c b/ldso/dynlink.c index 8707ae1c..3b57c07f 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -362,19 +362,14 @@ static struct symdef get_lfs64(const char *name) "pwritev\0readdir\0scandir\0sendfile\0setrlimit\0" "stat\0statfs\0statvfs\0tmpfile\0truncate\0versionsort\0" "__fxstat\0__fxstatat\0__lxstat\0__xstat\0"; - size_t l; - char buf[17]; - for (l=0; name[l]; l++) { - if (l >= sizeof buf) goto nomatch; - buf[l] = name[l]; - } if (!strcmp(name, "readdir64_r")) return find_sym(&ldso, "readdir_r", 1); - if (l<2 || name[l-2]!='6' || name[l-1]!='4') + size_t l = strnlen(name, 18); + if (l<2 || name[l-2]!='6' || name[l-1]!='4' || name[l]) goto nomatch; - buf[l-=2] = 0; for (p=lfs64_list; *p; p++) { - if (!strcmp(buf, p)) return find_sym(&ldso, buf, 1); + if (!strncmp(name, p, l-2) && !p[l-2]) + return find_sym(&ldso, p, 1); while (*p) p++; } nomatch: ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] [PATCH] dynlink: fix get_lfs64() with posix_fallocate64 2024-06-22 1:06 ` Rich Felker @ 2024-06-22 18:37 ` Florian Ziesche 0 siblings, 0 replies; 4+ messages in thread From: Florian Ziesche @ 2024-06-22 18:37 UTC (permalink / raw) To: Rich Felker; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 1660 bytes --] Yep, this works for me. > On 22. Jun 2024, at 03:06, Rich Felker <dalias@libc.org> wrote: > > On Thu, May 23, 2024 at 04:28:03PM -0400, Rich Felker wrote: >> On Fri, May 10, 2024 at 06:01:58PM +0200, Florian Ziesche wrote: >>> Hi, >>> >>> this patch increases the buffer size by one in get_lfs64() so that it >>> works with posix_fallocate64. >>> "posix_fallocate64" is 17 characters long, so 16 is one too short. >>> >>> Simplified example: >>> before: https://compiler-explorer.com/z/4qcPhcaWr >>> after: https://compiler-explorer.com/z/scGvhddKW >>> >>> --- >>> diff --git a/ldso/dynlink.c b/ldso/dynlink.c >>> index 42687da2..8707ae1c 100644 >>> --- a/ldso/dynlink.c >>> +++ b/ldso/dynlink.c >>> @@ -363,7 +363,7 @@ static struct symdef get_lfs64(const char *name) >>> "stat\0statfs\0statvfs\0tmpfile\0truncate\0versionsort\0" >>> "__fxstat\0__fxstatat\0__lxstat\0__xstat\0"; >>> size_t l; >>> - char buf[16]; >>> + char buf[17]; >>> for (l=0; name[l]; l++) { >>> if (l >= sizeof buf) goto nomatch; >>> buf[l] = name[l]; >> >> Thanks for catching this. I questioned whether the 17 is sufficient, >> but indeed the buffer is never nul terminated except when removing the >> 64, so it should be ok. >> >> I think I'll apply your patch as a direct fix, but the whole copying >> operation is unnecessary and I'll probably remove it. It works just as >> well to strncmp and pass p instead of buf to find_sym, so that a >> mutable copy of the name to lookup is never needed. > > Applied. Does the attached look ok to get rid of the copy entirely? It > passed basic smoke testing here. > > <get_lfs64.diff> [-- Attachment #2: Type: text/html, Size: 5420 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-06-22 19:35 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-05-10 16:01 [musl] [PATCH] dynlink: fix get_lfs64() with posix_fallocate64 Florian Ziesche 2024-05-23 20:28 ` Rich Felker 2024-06-22 1:06 ` Rich Felker 2024-06-22 18:37 ` Florian Ziesche
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).