All of lore.kernel.org
 help / color / mirror / Atom feed
* [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
@ 2022-05-23 13:34 Jiaqing Zhao
  2022-05-23 13:59 ` Alexander Kanavin
  0 siblings, 1 reply; 7+ messages in thread
From: Jiaqing Zhao @ 2022-05-23 13:34 UTC (permalink / raw)
  To: openembedded-core; +Cc: Jiaqing Zhao

Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
to fix musl build, but it changes the value here as pointer arithmetic
is type-depended in C. This patch corrects the behavior by adding an
extra cast to struct dirent*.

Also changes the patch's Upstream-Status to Inappropriate as it's musl-
specific.

Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
---
 ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
index 028f50b243..9ebff9825a 100644
--- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
+++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
@@ -1,4 +1,4 @@
-From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
+From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
 From: Khem Raj <raj.khem@gmail.com>
 Date: Fri, 21 Jan 2022 15:15:11 -0800
 Subject: [PATCH] pass correct parameters to getdents64
@@ -12,16 +12,16 @@ Fixes
         n = getdents64(fd, &buffer, sizeof(buffer));
                            ^~~~~~~
 
-Upstream-Status: Pending
+Upstream-Status: Inappropriate [musl specific]
 Signed-off-by: Khem Raj <raj.khem@gmail.com>
-
+Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
 ---
  src/basic/recurse-dir.c | 2 +-
  src/basic/stat-util.c   | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
-index efa1797b7b..797285e3be 100644
+index efa1797b7b..03ff10ebe9 100644
 --- a/src/basic/recurse-dir.c
 +++ b/src/basic/recurse-dir.c
 @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
@@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
                  assert(bs > de->buffer_size);
  
 -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
-+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
                  if (n < 0)
                          return -errno;
                  if (n == 0)
@@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
          if (n < 0)
                  return -errno;
  
+-- 
+2.34.1
+
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
  2022-05-23 13:34 [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch Jiaqing Zhao
@ 2022-05-23 13:59 ` Alexander Kanavin
  2022-05-23 14:21   ` Jiaqing Zhao
  0 siblings, 1 reply; 7+ messages in thread
From: Alexander Kanavin @ 2022-05-23 13:59 UTC (permalink / raw)
  To: Jiaqing Zhao; +Cc: OE-core

Inappropriate still means the issue needs to be reported to upstream,
can you do this please?

Alex

On Mon, 23 May 2022 at 15:34, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>
> Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
> to fix musl build, but it changes the value here as pointer arithmetic
> is type-depended in C. This patch corrects the behavior by adding an
> extra cast to struct dirent*.
>
> Also changes the patch's Upstream-Status to Inappropriate as it's musl-
> specific.
>
> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> ---
>  ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> index 028f50b243..9ebff9825a 100644
> --- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> +++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> @@ -1,4 +1,4 @@
> -From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
> +From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
>  From: Khem Raj <raj.khem@gmail.com>
>  Date: Fri, 21 Jan 2022 15:15:11 -0800
>  Subject: [PATCH] pass correct parameters to getdents64
> @@ -12,16 +12,16 @@ Fixes
>          n = getdents64(fd, &buffer, sizeof(buffer));
>                             ^~~~~~~
>
> -Upstream-Status: Pending
> +Upstream-Status: Inappropriate [musl specific]
>  Signed-off-by: Khem Raj <raj.khem@gmail.com>
> -
> +Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>  ---
>   src/basic/recurse-dir.c | 2 +-
>   src/basic/stat-util.c   | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
>  diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
> -index efa1797b7b..797285e3be 100644
> +index efa1797b7b..03ff10ebe9 100644
>  --- a/src/basic/recurse-dir.c
>  +++ b/src/basic/recurse-dir.c
>  @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
> @@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
>                   assert(bs > de->buffer_size);
>
>  -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
> -+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
> ++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
>                   if (n < 0)
>                           return -errno;
>                   if (n == 0)
> @@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
>           if (n < 0)
>                   return -errno;
>
> +--
> +2.34.1
> +
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#166021): https://lists.openembedded.org/g/openembedded-core/message/166021
> Mute This Topic: https://lists.openembedded.org/mt/91288052/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
  2022-05-23 13:59 ` Alexander Kanavin
@ 2022-05-23 14:21   ` Jiaqing Zhao
  2022-05-23 14:29     ` Alexander Kanavin
  2022-05-23 16:22     ` Khem Raj
  0 siblings, 2 replies; 7+ messages in thread
From: Jiaqing Zhao @ 2022-05-23 14:21 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: OE-core

It's an musl-specific issue I believe.

man page defines it as [1]
	ssize_t getdents64(int fd, void *dirp, size_t count)

But in musl, it's an alias of getdents [2]
	int getdents(int fd, struct dirent *buf, size_t len)

Shall we report to musl instead of systemd?

[1] https://man7.org/linux/man-pages/man2/getdents.2.html
[2] https://elixir.bootlin.com/musl/v1.2.3/source/src/linux/getdents.c

On 2022-05-23 21:59, Alexander Kanavin wrote:
> Inappropriate still means the issue needs to be reported to upstream,
> can you do this please?
> 
> Alex
> 
> On Mon, 23 May 2022 at 15:34, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>>
>> Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
>> to fix musl build, but it changes the value here as pointer arithmetic
>> is type-depended in C. This patch corrects the behavior by adding an
>> extra cast to struct dirent*.
>>
>> Also changes the patch's Upstream-Status to Inappropriate as it's musl-
>> specific.
>>
>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>> ---
>>  ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>> index 028f50b243..9ebff9825a 100644
>> --- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>> +++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>> @@ -1,4 +1,4 @@
>> -From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
>> +From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
>>  From: Khem Raj <raj.khem@gmail.com>
>>  Date: Fri, 21 Jan 2022 15:15:11 -0800
>>  Subject: [PATCH] pass correct parameters to getdents64
>> @@ -12,16 +12,16 @@ Fixes
>>          n = getdents64(fd, &buffer, sizeof(buffer));
>>                             ^~~~~~~
>>
>> -Upstream-Status: Pending
>> +Upstream-Status: Inappropriate [musl specific]
>>  Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> -
>> +Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>>  ---
>>   src/basic/recurse-dir.c | 2 +-
>>   src/basic/stat-util.c   | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>>  diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
>> -index efa1797b7b..797285e3be 100644
>> +index efa1797b7b..03ff10ebe9 100644
>>  --- a/src/basic/recurse-dir.c
>>  +++ b/src/basic/recurse-dir.c
>>  @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
>> @@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
>>                   assert(bs > de->buffer_size);
>>
>>  -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
>> -+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
>> ++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
>>                   if (n < 0)
>>                           return -errno;
>>                   if (n == 0)
>> @@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
>>           if (n < 0)
>>                   return -errno;
>>
>> +--
>> +2.34.1
>> +
>> --
>> 2.34.1
>>
>>
>>
>>
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#166023): https://lists.openembedded.org/g/openembedded-core/message/166023
>> Mute This Topic: https://lists.openembedded.org/mt/91288052/6787970
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [jiaqing.zhao@linux.intel.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
  2022-05-23 14:21   ` Jiaqing Zhao
@ 2022-05-23 14:29     ` Alexander Kanavin
  2022-05-23 16:22     ` Khem Raj
  1 sibling, 0 replies; 7+ messages in thread
From: Alexander Kanavin @ 2022-05-23 14:29 UTC (permalink / raw)
  To: Jiaqing Zhao; +Cc: OE-core

I guess so, please do.

Alex

On Mon, 23 May 2022 at 16:21, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>
> It's an musl-specific issue I believe.
>
> man page defines it as [1]
>         ssize_t getdents64(int fd, void *dirp, size_t count)
>
> But in musl, it's an alias of getdents [2]
>         int getdents(int fd, struct dirent *buf, size_t len)
>
> Shall we report to musl instead of systemd?
>
> [1] https://man7.org/linux/man-pages/man2/getdents.2.html
> [2] https://elixir.bootlin.com/musl/v1.2.3/source/src/linux/getdents.c
>
> On 2022-05-23 21:59, Alexander Kanavin wrote:
> > Inappropriate still means the issue needs to be reported to upstream,
> > can you do this please?
> >
> > Alex
> >
> > On Mon, 23 May 2022 at 15:34, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
> >>
> >> Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
> >> to fix musl build, but it changes the value here as pointer arithmetic
> >> is type-depended in C. This patch corrects the behavior by adding an
> >> extra cast to struct dirent*.
> >>
> >> Also changes the patch's Upstream-Status to Inappropriate as it's musl-
> >> specific.
> >>
> >> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> >> ---
> >>  ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
> >>  1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> >> index 028f50b243..9ebff9825a 100644
> >> --- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> >> +++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> >> @@ -1,4 +1,4 @@
> >> -From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
> >> +From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
> >>  From: Khem Raj <raj.khem@gmail.com>
> >>  Date: Fri, 21 Jan 2022 15:15:11 -0800
> >>  Subject: [PATCH] pass correct parameters to getdents64
> >> @@ -12,16 +12,16 @@ Fixes
> >>          n = getdents64(fd, &buffer, sizeof(buffer));
> >>                             ^~~~~~~
> >>
> >> -Upstream-Status: Pending
> >> +Upstream-Status: Inappropriate [musl specific]
> >>  Signed-off-by: Khem Raj <raj.khem@gmail.com>
> >> -
> >> +Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> >>  ---
> >>   src/basic/recurse-dir.c | 2 +-
> >>   src/basic/stat-util.c   | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >>  diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
> >> -index efa1797b7b..797285e3be 100644
> >> +index efa1797b7b..03ff10ebe9 100644
> >>  --- a/src/basic/recurse-dir.c
> >>  +++ b/src/basic/recurse-dir.c
> >>  @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
> >> @@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
> >>                   assert(bs > de->buffer_size);
> >>
> >>  -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
> >> -+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
> >> ++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
> >>                   if (n < 0)
> >>                           return -errno;
> >>                   if (n == 0)
> >> @@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
> >>           if (n < 0)
> >>                   return -errno;
> >>
> >> +--
> >> +2.34.1
> >> +
> >> --
> >> 2.34.1
> >>
> >>
> >>
> >>
> >>
> >>
> >> -=-=-=-=-=-=-=-=-=-=-=-
> >> Links: You receive all messages sent to this group.
> >> View/Reply Online (#166023): https://lists.openembedded.org/g/openembedded-core/message/166023
> >> Mute This Topic: https://lists.openembedded.org/mt/91288052/6787970
> >> Group Owner: openembedded-core+owner@lists.openembedded.org
> >> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [jiaqing.zhao@linux.intel.com]
> >> -=-=-=-=-=-=-=-=-=-=-=-
> >>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
  2022-05-23 14:21   ` Jiaqing Zhao
  2022-05-23 14:29     ` Alexander Kanavin
@ 2022-05-23 16:22     ` Khem Raj
  2022-05-24  8:15       ` Jiaqing Zhao
  2022-05-28 12:43       ` Jiaqing Zhao
  1 sibling, 2 replies; 7+ messages in thread
From: Khem Raj @ 2022-05-23 16:22 UTC (permalink / raw)
  To: Jiaqing Zhao; +Cc: Alexander Kanavin, OE-core

On Mon, May 23, 2022 at 7:21 AM Jiaqing Zhao
<jiaqing.zhao@linux.intel.com> wrote:
>
> It's an musl-specific issue I believe.
>
> man page defines it as [1]
>         ssize_t getdents64(int fd, void *dirp, size_t count)
>
> But in musl, it's an alias of getdents [2]
>         int getdents(int fd, struct dirent *buf, size_t len)
>
> Shall we report to musl instead of systemd?
>
> [1] https://man7.org/linux/man-pages/man2/getdents.2.html
> [2] https://elixir.bootlin.com/musl/v1.2.3/source/src/linux/getdents.c
>

I think it is worth reporting at both places.

Here the data buffer is filled with packed dirent
structures, not regular dirent structure, therefore a void* argument
like glibc is doing
probably makes more sense. But we have to reason it with musl community as well.

> On 2022-05-23 21:59, Alexander Kanavin wrote:
> > Inappropriate still means the issue needs to be reported to upstream,
> > can you do this please?
> >
> > Alex
> >
> > On Mon, 23 May 2022 at 15:34, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
> >>
> >> Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
> >> to fix musl build, but it changes the value here as pointer arithmetic
> >> is type-depended in C. This patch corrects the behavior by adding an
> >> extra cast to struct dirent*.
> >>
> >> Also changes the patch's Upstream-Status to Inappropriate as it's musl-
> >> specific.
> >>
> >> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> >> ---
> >>  ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
> >>  1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> >> index 028f50b243..9ebff9825a 100644
> >> --- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> >> +++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
> >> @@ -1,4 +1,4 @@
> >> -From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
> >> +From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
> >>  From: Khem Raj <raj.khem@gmail.com>
> >>  Date: Fri, 21 Jan 2022 15:15:11 -0800
> >>  Subject: [PATCH] pass correct parameters to getdents64
> >> @@ -12,16 +12,16 @@ Fixes
> >>          n = getdents64(fd, &buffer, sizeof(buffer));
> >>                             ^~~~~~~
> >>
> >> -Upstream-Status: Pending
> >> +Upstream-Status: Inappropriate [musl specific]
> >>  Signed-off-by: Khem Raj <raj.khem@gmail.com>
> >> -
> >> +Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
> >>  ---
> >>   src/basic/recurse-dir.c | 2 +-
> >>   src/basic/stat-util.c   | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >>  diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
> >> -index efa1797b7b..797285e3be 100644
> >> +index efa1797b7b..03ff10ebe9 100644
> >>  --- a/src/basic/recurse-dir.c
> >>  +++ b/src/basic/recurse-dir.c
> >>  @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
> >> @@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
> >>                   assert(bs > de->buffer_size);
> >>
> >>  -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
> >> -+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
> >> ++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
> >>                   if (n < 0)
> >>                           return -errno;
> >>                   if (n == 0)
> >> @@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
> >>           if (n < 0)
> >>                   return -errno;
> >>
> >> +--
> >> +2.34.1
> >> +
> >> --
> >> 2.34.1
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#166033): https://lists.openembedded.org/g/openembedded-core/message/166033
> Mute This Topic: https://lists.openembedded.org/mt/91288052/1997914
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
  2022-05-23 16:22     ` Khem Raj
@ 2022-05-24  8:15       ` Jiaqing Zhao
  2022-05-28 12:43       ` Jiaqing Zhao
  1 sibling, 0 replies; 7+ messages in thread
From: Jiaqing Zhao @ 2022-05-24  8:15 UTC (permalink / raw)
  To: Khem Raj; +Cc: Alexander Kanavin, OE-core

On 2022-05-24 00:22, Khem Raj wrote:
> On Mon, May 23, 2022 at 7:21 AM Jiaqing Zhao
> <jiaqing.zhao@linux.intel.com> wrote:
>>
>> It's an musl-specific issue I believe.
>>
>> man page defines it as [1]
>>         ssize_t getdents64(int fd, void *dirp, size_t count)
>>
>> But in musl, it's an alias of getdents [2]
>>         int getdents(int fd, struct dirent *buf, size_t len)
>>
>> Shall we report to musl instead of systemd?
>>
>> [1] https://man7.org/linux/man-pages/man2/getdents.2.html
>> [2] https://elixir.bootlin.com/musl/v1.2.3/source/src/linux/getdents.c
>>
> 
> I think it is worth reporting at both places.
> 
> Here the data buffer is filled with packed dirent
> structures, not regular dirent structure, therefore a void* argument
> like glibc is doing
> probably makes more sense. But we have to reason it with musl community as well.

May I ask where the packed dirent structure is defined? I cannot find it.
On my setup, both glibc and musl are not defining packed dirent.

I think the code in systemd is correct, but the getdents64 in musl does
not meet the standard.

I've submitted a musl patch at https://www.openwall.com/lists/musl/2022/05/24/3

>> On 2022-05-23 21:59, Alexander Kanavin wrote:
>>> Inappropriate still means the issue needs to be reported to upstream,
>>> can you do this please?
>>>
>>> Alex
>>>
>>> On Mon, 23 May 2022 at 15:34, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>>>>
>>>> Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
>>>> to fix musl build, but it changes the value here as pointer arithmetic
>>>> is type-depended in C. This patch corrects the behavior by adding an
>>>> extra cast to struct dirent*.
>>>>
>>>> Also changes the patch's Upstream-Status to Inappropriate as it's musl-
>>>> specific.
>>>>
>>>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>>>> ---
>>>>  ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
>>>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>>>> index 028f50b243..9ebff9825a 100644
>>>> --- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>>>> +++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>>>> @@ -1,4 +1,4 @@
>>>> -From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
>>>> +From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
>>>>  From: Khem Raj <raj.khem@gmail.com>
>>>>  Date: Fri, 21 Jan 2022 15:15:11 -0800
>>>>  Subject: [PATCH] pass correct parameters to getdents64
>>>> @@ -12,16 +12,16 @@ Fixes
>>>>          n = getdents64(fd, &buffer, sizeof(buffer));
>>>>                             ^~~~~~~
>>>>
>>>> -Upstream-Status: Pending
>>>> +Upstream-Status: Inappropriate [musl specific]
>>>>  Signed-off-by: Khem Raj <raj.khem@gmail.com>
>>>> -
>>>> +Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>>>>  ---
>>>>   src/basic/recurse-dir.c | 2 +-
>>>>   src/basic/stat-util.c   | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>>  diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
>>>> -index efa1797b7b..797285e3be 100644
>>>> +index efa1797b7b..03ff10ebe9 100644
>>>>  --- a/src/basic/recurse-dir.c
>>>>  +++ b/src/basic/recurse-dir.c
>>>>  @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
>>>> @@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
>>>>                   assert(bs > de->buffer_size);
>>>>
>>>>  -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
>>>> -+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
>>>> ++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
>>>>                   if (n < 0)
>>>>                           return -errno;
>>>>                   if (n == 0)
>>>> @@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
>>>>           if (n < 0)
>>>>                   return -errno;
>>>>
>>>> +--
>>>> +2.34.1
>>>> +
>>>> --
>>>> 2.34.1
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>>
>>
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#166039): https://lists.openembedded.org/g/openembedded-core/message/166039
>> Mute This Topic: https://lists.openembedded.org/mt/91288052/6787970
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [jiaqing.zhao@linux.intel.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch
  2022-05-23 16:22     ` Khem Raj
  2022-05-24  8:15       ` Jiaqing Zhao
@ 2022-05-28 12:43       ` Jiaqing Zhao
  1 sibling, 0 replies; 7+ messages in thread
From: Jiaqing Zhao @ 2022-05-28 12:43 UTC (permalink / raw)
  To: Khem Raj; +Cc: Alexander Kanavin, OE-core

On 2022-05-24 00:22, Khem Raj wrote:
> On Mon, May 23, 2022 at 7:21 AM Jiaqing Zhao
> <jiaqing.zhao@linux.intel.com> wrote:
>>
>> It's an musl-specific issue I believe.
>>
>> man page defines it as [1]
>>         ssize_t getdents64(int fd, void *dirp, size_t count)
>>
>> But in musl, it's an alias of getdents [2]
>>         int getdents(int fd, struct dirent *buf, size_t len)
>>
>> Shall we report to musl instead of systemd?
>>
>> [1] https://man7.org/linux/man-pages/man2/getdents.2.html
>> [2] https://elixir.bootlin.com/musl/v1.2.3/source/src/linux/getdents.c
>>
> 
> I think it is worth reporting at both places.
> 
> Here the data buffer is filled with packed dirent
> structures, not regular dirent structure, therefore a void* argument
> like glibc is doing
> probably makes more sense. But we have to reason it with musl community as well.

musl community replied me the reason they made getdents64 an alias of getdents at
https://www.openwall.com/lists/musl/2022/05/24/5

While systemd community will absolutely ignore this issue as it's musl-specific,
systemd only focuses on glibc.
https://github.com/systemd/systemd/blob/01ae74c8c7f364dd3a1d71a43af00c34f531d3f7/docs/CODING_STYLE.md?plain=1#L200

>> On 2022-05-23 21:59, Alexander Kanavin wrote:
>>> Inappropriate still means the issue needs to be reported to upstream,
>>> can you do this please?
>>>
>>> Alex
>>>
>>> On Mon, 23 May 2022 at 15:34, Jiaqing Zhao <jiaqing.zhao@linux.intel.com> wrote:
>>>>
>>>> Current patch removes the uint8_t* cast in src/basic/recurse-dir.c:57
>>>> to fix musl build, but it changes the value here as pointer arithmetic
>>>> is type-depended in C. This patch corrects the behavior by adding an
>>>> extra cast to struct dirent*.
>>>>
>>>> Also changes the patch's Upstream-Status to Inappropriate as it's musl-
>>>> specific.
>>>>
>>>> Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>>>> ---
>>>>  ...0001-pass-correct-parameters-to-getdents64.patch | 13 ++++++++-----
>>>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>>>> index 028f50b243..9ebff9825a 100644
>>>> --- a/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>>>> +++ b/meta/recipes-core/systemd/systemd/0001-pass-correct-parameters-to-getdents64.patch
>>>> @@ -1,4 +1,4 @@
>>>> -From 8c8899b4641125cfe8e7baee32e5c5f452545d2c Mon Sep 17 00:00:00 2001
>>>> +From dab02796780f00d689cc1c7a0ba81abe7c5f28d0 Mon Sep 17 00:00:00 2001
>>>>  From: Khem Raj <raj.khem@gmail.com>
>>>>  Date: Fri, 21 Jan 2022 15:15:11 -0800
>>>>  Subject: [PATCH] pass correct parameters to getdents64
>>>> @@ -12,16 +12,16 @@ Fixes
>>>>          n = getdents64(fd, &buffer, sizeof(buffer));
>>>>                             ^~~~~~~
>>>>
>>>> -Upstream-Status: Pending
>>>> +Upstream-Status: Inappropriate [musl specific]
>>>>  Signed-off-by: Khem Raj <raj.khem@gmail.com>
>>>> -
>>>> +Signed-off-by: Jiaqing Zhao <jiaqing.zhao@linux.intel.com>
>>>>  ---
>>>>   src/basic/recurse-dir.c | 2 +-
>>>>   src/basic/stat-util.c   | 2 +-
>>>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>>  diff --git a/src/basic/recurse-dir.c b/src/basic/recurse-dir.c
>>>> -index efa1797b7b..797285e3be 100644
>>>> +index efa1797b7b..03ff10ebe9 100644
>>>>  --- a/src/basic/recurse-dir.c
>>>>  +++ b/src/basic/recurse-dir.c
>>>>  @@ -54,7 +54,7 @@ int readdir_all(int dir_fd,
>>>> @@ -29,7 +29,7 @@ index efa1797b7b..797285e3be 100644
>>>>                   assert(bs > de->buffer_size);
>>>>
>>>>  -                n = getdents64(dir_fd, (uint8_t*) de->buffer + de->buffer_size, bs - de->buffer_size);
>>>> -+                n = getdents64(dir_fd, de->buffer + de->buffer_size, bs - de->buffer_size);
>>>> ++                n = getdents64(dir_fd, (struct dirent*)((uint8_t*) de->buffer + de->buffer_size), bs - de->buffer_size);
>>>>                   if (n < 0)
>>>>                           return -errno;
>>>>                   if (n == 0)
>>>> @@ -46,3 +46,6 @@ index c2269844f8..7cd6c7fa42 100644
>>>>           if (n < 0)
>>>>                   return -errno;
>>>>
>>>> +--
>>>> +2.34.1
>>>> +
>>>> --
>>>> 2.34.1
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#166033): https://lists.openembedded.org/g/openembedded-core/message/166033
>> Mute This Topic: https://lists.openembedded.org/mt/91288052/1997914
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-05-28 12:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-23 13:34 [oe-core] [PATCH] systemd: Correct 0001-pass-correct-parameters-to-getdents64.patch Jiaqing Zhao
2022-05-23 13:59 ` Alexander Kanavin
2022-05-23 14:21   ` Jiaqing Zhao
2022-05-23 14:29     ` Alexander Kanavin
2022-05-23 16:22     ` Khem Raj
2022-05-24  8:15       ` Jiaqing Zhao
2022-05-28 12:43       ` Jiaqing Zhao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.