[v2] kheaders: Follow symlinks to source files.

[PATCH v2] kheaders: Follow symlinks to source files.

Posted by Yifan Hong 2 years, 8 months ago

When the kernel is built inside a sandbox container,
a forest of symlinks to the source files may be
created in the container. In this case, the generated
kheaders.tar.xz should follow these symlinks
to access the source files, instead of packing
the symlinks themselves.

Signed-off-by: Yifan Hong <elsk@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>
---
 kernel/gen_kheaders.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
index 1ef9a87511f5..70a75a9a7535 100755
--- a/kernel/gen_kheaders.sh
+++ b/kernel/gen_kheaders.sh
@@ -87,7 +87,7 @@ find $cpio_dir -type f -print0 |
 # pre-sorted, as --sort=name might not be available.
 find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
     tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
-    --owner=0 --group=0 --numeric-owner --no-recursion \
+    --owner=0 --group=0 --numeric-owner --no-recursion --dereference \
     -I $XZ -cf $tarfile -C $cpio_dir/ -T - > /dev/null
 
 echo $headers_md5 > kernel/kheaders.md5
-- 
2.40.0.634.g4ca3ef3211-goog

Re: [PATCH v2] kheaders: Follow symlinks to source files.

Posted by Masahiro Yamada 2 years, 7 months ago

On Thu, Apr 20, 2023 at 10:01 AM Yifan Hong <elsk@google.com> wrote:
>
> When the kernel is built inside a sandbox container,
> a forest of symlinks to the source files may be
> created in the container. In this case, the generated
> kheaders.tar.xz should follow these symlinks
> to access the source files, instead of packing
> the symlinks themselves.
>
> Signed-off-by: Yifan Hong <elsk@google.com>
> Reviewed-by: Matthias Maennich <maennich@google.com>



Please teach me how to reproduce your problem.



I built the kernel inside a container,
but I do not see any difference.



We have some symbolic links in include/,
but those reference relative paths.
How building the kernel in a container
makes a difference?


masahiro@bc5c2c958b53:/tmp/foo$ tar Jxf kheaders_data.tar.xz
masahiro@bc5c2c958b53:/tmp/foo$ find . -type l
./include/dt-bindings/clock/qcom,dispcc-sm8350.h
./include/dt-bindings/clock/qcom,dispcc-sm8150.h
./include/dt-bindings/input/linux-event-codes.h
masahiro@bc5c2c958b53:/tmp/foo$ find . -type l | xargs ls -l
lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
./include/dt-bindings/clock/qcom,dispcc-sm8150.h ->
qcom,dispcc-sm8250.h
lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
./include/dt-bindings/clock/qcom,dispcc-sm8350.h ->
qcom,dispcc-sm8250.h
lrwxrwxrwx 1 masahiro masahiro 36 Apr 23 11:47
./include/dt-bindings/input/linux-event-codes.h ->
../../uapi/linux/input-event-codes.h












> ---
>  kernel/gen_kheaders.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
> index 1ef9a87511f5..70a75a9a7535 100755
> --- a/kernel/gen_kheaders.sh
> +++ b/kernel/gen_kheaders.sh
> @@ -87,7 +87,7 @@ find $cpio_dir -type f -print0 |
>  # pre-sorted, as --sort=name might not be available.
>  find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
>      tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> -    --owner=0 --group=0 --numeric-owner --no-recursion \
> +    --owner=0 --group=0 --numeric-owner --no-recursion --dereference \
>      -I $XZ -cf $tarfile -C $cpio_dir/ -T - > /dev/null
>
>  echo $headers_md5 > kernel/kheaders.md5
> --
> 2.40.0.634.g4ca3ef3211-goog
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2] kheaders: Follow symlinks to source files.

Posted by Yifan Hong 2 years, 7 months ago

On Sun, Apr 23, 2023 at 4:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Apr 20, 2023 at 10:01 AM Yifan Hong <elsk@google.com> wrote:
> >
> > When the kernel is built inside a sandbox container,
> > a forest of symlinks to the source files may be
> > created in the container. In this case, the generated
> > kheaders.tar.xz should follow these symlinks
> > to access the source files, instead of packing
> > the symlinks themselves.
> >
> > Signed-off-by: Yifan Hong <elsk@google.com>
> > Reviewed-by: Matthias Maennich <maennich@google.com>
>
>
>
> Please teach me how to reproduce your problem.

Thanks for your reply. When I say "container", I mean a forest of
symlinks that point to the original source files. Here are the steps I
do to mimic the sandbox that the container program creates.

On an x86 machine, I have a linux tree checkout at /mnt/sdb/linux, and
an empty directory at /mnt/sdb/linux2. Then I **created this fake
sandbox** in /mnt/sdb/linux2 by doing:
```
$ cd /mnt/sdb/linux
$ find . -type d -exec mkdir -p /mnt/sdb/linux2/{} \;
$ find . -type f -exec ln -sf /mnt/sdb/linux/{} /mnt/sdb/linux2/{} \;
```

Then, I cross compile an arm64 kernel with tinyconfig. Here are the
steps I take:
```
export ARCH=arm64
export SUBARCH=arm
export CROSS_COMPILE=/home/elsk/toolchains/aarch64--glibc--stable-2022.08-1/bin/aarch64-linux-
# downloaded from https://toolchains.bootlin.com/
make tinyconfig; make menuconfig # then select CONFIG_SYSFS and CONFIG_IKHEADERS
make -j64
```

This will show that kheaders_data.tar.xz contains the symlinks, e.g. in my case:
```
$ tar tvf ./kernel/kheaders_data.tar.xz | head
[...]
lrwxrwxrwx 0/0               0 2023-04-24 11:19
./arch/arm64/include/asm/acenv.h ->
/mnt/sdb/linux/./arch/arm64/include/asm/acenv.h
```

However, the issue for me is that these symlinks are no longer useful
under sysfs when I am booting the kernel on the target arm64 machine,
because it is a different machine (arm64) than the machine (x86) where
I compiled this kernel.

>
>
>
> I built the kernel inside a container,
> but I do not see any difference.
>
>
>
> We have some symbolic links in include/,
> but those reference relative paths.
> How building the kernel in a container
> makes a difference?

Please see above; the container contains a forest of symlinks pointing
to the original source tree. Then `make` is executed in that container
directory.

>
>
> masahiro@bc5c2c958b53:/tmp/foo$ tar Jxf kheaders_data.tar.xz
> masahiro@bc5c2c958b53:/tmp/foo$ find . -type l
> ./include/dt-bindings/clock/qcom,dispcc-sm8350.h
> ./include/dt-bindings/clock/qcom,dispcc-sm8150.h
> ./include/dt-bindings/input/linux-event-codes.h
> masahiro@bc5c2c958b53:/tmp/foo$ find . -type l | xargs ls -l
> lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
> ./include/dt-bindings/clock/qcom,dispcc-sm8150.h ->
> qcom,dispcc-sm8250.h
> lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
> ./include/dt-bindings/clock/qcom,dispcc-sm8350.h ->
> qcom,dispcc-sm8250.h
> lrwxrwxrwx 1 masahiro masahiro 36 Apr 23 11:47
> ./include/dt-bindings/input/linux-event-codes.h ->
> ../../uapi/linux/input-event-codes.h
>
>
>
>
>
>
>
>
>
>
>
>
> > ---
> >  kernel/gen_kheaders.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
> > index 1ef9a87511f5..70a75a9a7535 100755
> > --- a/kernel/gen_kheaders.sh
> > +++ b/kernel/gen_kheaders.sh
> > @@ -87,7 +87,7 @@ find $cpio_dir -type f -print0 |
> >  # pre-sorted, as --sort=name might not be available.
> >  find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
> >      tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> > -    --owner=0 --group=0 --numeric-owner --no-recursion \
> > +    --owner=0 --group=0 --numeric-owner --no-recursion --dereference \
> >      -I $XZ -cf $tarfile -C $cpio_dir/ -T - > /dev/null
> >
> >  echo $headers_md5 > kernel/kheaders.md5
> > --
> > 2.40.0.634.g4ca3ef3211-goog
> >
>
>
> --
> Best Regards
> Masahiro Yamada
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>

Re: [PATCH v2] kheaders: Follow symlinks to source files.

Posted by Yifan Hong 2 years, 7 months ago

On Mon, Apr 24, 2023 at 11:32 AM Yifan Hong <elsk@google.com> wrote:
>
> On Sun, Apr 23, 2023 at 4:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Thu, Apr 20, 2023 at 10:01 AM Yifan Hong <elsk@google.com> wrote:
> > >
> > > When the kernel is built inside a sandbox container,
> > > a forest of symlinks to the source files may be
> > > created in the container. In this case, the generated
> > > kheaders.tar.xz should follow these symlinks
> > > to access the source files, instead of packing
> > > the symlinks themselves.
> > >
> > > Signed-off-by: Yifan Hong <elsk@google.com>
> > > Reviewed-by: Matthias Maennich <maennich@google.com>
> >
> >
> >
> > Please teach me how to reproduce your problem.
>
> Thanks for your reply. When I say "container", I mean a forest of
> symlinks that point to the original source files. Here are the steps I
> do to mimic the sandbox that the container program creates.
>
> On an x86 machine, I have a linux tree checkout at /mnt/sdb/linux, and
> an empty directory at /mnt/sdb/linux2. Then I **created this fake
> sandbox** in /mnt/sdb/linux2 by doing:
> ```
> $ cd /mnt/sdb/linux
> $ find . -type d -exec mkdir -p /mnt/sdb/linux2/{} \;
> $ find . -type f -exec ln -sf /mnt/sdb/linux/{} /mnt/sdb/linux2/{} \;
> ```
>
> Then, I cross compile an arm64 kernel with tinyconfig. Here are the
> steps I take:
> ```
> export ARCH=arm64
> export SUBARCH=arm
> export CROSS_COMPILE=/home/elsk/toolchains/aarch64--glibc--stable-2022.08-1/bin/aarch64-linux-
> # downloaded from https://toolchains.bootlin.com/
> make tinyconfig; make menuconfig # then select CONFIG_SYSFS and CONFIG_IKHEADERS
> make -j64
> ```
>
> This will show that kheaders_data.tar.xz contains the symlinks, e.g. in my case:
> ```
> $ tar tvf ./kernel/kheaders_data.tar.xz | head
> [...]
> lrwxrwxrwx 0/0               0 2023-04-24 11:19
> ./arch/arm64/include/asm/acenv.h ->
> /mnt/sdb/linux/./arch/arm64/include/asm/acenv.h
> ```
>
> However, the issue for me is that these symlinks are no longer useful
> under sysfs when I am booting the kernel on the target arm64 machine,
> because it is a different machine (arm64) than the machine (x86) where
> I compiled this kernel.
>
> >
> >
> >
> > I built the kernel inside a container,
> > but I do not see any difference.
> >
> >
> >
> > We have some symbolic links in include/,
> > but those reference relative paths.
> > How building the kernel in a container
> > makes a difference?
>
> Please see above; the container contains a forest of symlinks pointing
> to the original source tree. Then `make` is executed in that container
> directory.
>
> >
> >
> > masahiro@bc5c2c958b53:/tmp/foo$ tar Jxf kheaders_data.tar.xz
> > masahiro@bc5c2c958b53:/tmp/foo$ find . -type l
> > ./include/dt-bindings/clock/qcom,dispcc-sm8350.h
> > ./include/dt-bindings/clock/qcom,dispcc-sm8150.h
> > ./include/dt-bindings/input/linux-event-codes.h
> > masahiro@bc5c2c958b53:/tmp/foo$ find . -type l | xargs ls -l
> > lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
> > ./include/dt-bindings/clock/qcom,dispcc-sm8150.h ->
> > qcom,dispcc-sm8250.h
> > lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
> > ./include/dt-bindings/clock/qcom,dispcc-sm8350.h ->
> > qcom,dispcc-sm8250.h
> > lrwxrwxrwx 1 masahiro masahiro 36 Apr 23 11:47
> > ./include/dt-bindings/input/linux-event-codes.h ->
> > ../../uapi/linux/input-event-codes.h
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > > ---
> > >  kernel/gen_kheaders.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
> > > index 1ef9a87511f5..70a75a9a7535 100755
> > > --- a/kernel/gen_kheaders.sh
> > > +++ b/kernel/gen_kheaders.sh
> > > @@ -87,7 +87,7 @@ find $cpio_dir -type f -print0 |
> > >  # pre-sorted, as --sort=name might not be available.
> > >  find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
> > >      tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> > > -    --owner=0 --group=0 --numeric-owner --no-recursion \
> > > +    --owner=0 --group=0 --numeric-owner --no-recursion --dereference \
> > >      -I $XZ -cf $tarfile -C $cpio_dir/ -T - > /dev/null
> > >
> > >  echo $headers_md5 > kernel/kheaders.md5
> > > --
> > > 2.40.0.634.g4ca3ef3211-goog
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
> > --
> > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
> >

Gently nagging. Is there anything I need to fix or do to improve this
patch so it may be accepted? Thank you!

Re: [PATCH v2] kheaders: Follow symlinks to source files.

Posted by Masahiro Yamada 2 years, 7 months ago

On Wed, May 3, 2023 at 12:17 AM Yifan Hong <elsk@google.com> wrote:
>
> On Mon, Apr 24, 2023 at 11:32 AM Yifan Hong <elsk@google.com> wrote:
> >
> > On Sun, Apr 23, 2023 at 4:53 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Thu, Apr 20, 2023 at 10:01 AM Yifan Hong <elsk@google.com> wrote:
> > > >
> > > > When the kernel is built inside a sandbox container,
> > > > a forest of symlinks to the source files may be
> > > > created in the container. In this case, the generated
> > > > kheaders.tar.xz should follow these symlinks
> > > > to access the source files, instead of packing
> > > > the symlinks themselves.
> > > >
> > > > Signed-off-by: Yifan Hong <elsk@google.com>
> > > > Reviewed-by: Matthias Maennich <maennich@google.com>
> > >
> > >
> > >
> > > Please teach me how to reproduce your problem.
> >
> > Thanks for your reply. When I say "container", I mean a forest of
> > symlinks that point to the original source files. Here are the steps I
> > do to mimic the sandbox that the container program creates.
> >
> > On an x86 machine, I have a linux tree checkout at /mnt/sdb/linux, and
> > an empty directory at /mnt/sdb/linux2. Then I **created this fake
> > sandbox** in /mnt/sdb/linux2 by doing:
> > ```
> > $ cd /mnt/sdb/linux
> > $ find . -type d -exec mkdir -p /mnt/sdb/linux2/{} \;
> > $ find . -type f -exec ln -sf /mnt/sdb/linux/{} /mnt/sdb/linux2/{} \;
> > ```
> >
> > Then, I cross compile an arm64 kernel with tinyconfig. Here are the
> > steps I take:
> > ```
> > export ARCH=arm64
> > export SUBARCH=arm
> > export CROSS_COMPILE=/home/elsk/toolchains/aarch64--glibc--stable-2022.08-1/bin/aarch64-linux-
> > # downloaded from https://toolchains.bootlin.com/
> > make tinyconfig; make menuconfig # then select CONFIG_SYSFS and CONFIG_IKHEADERS
> > make -j64
> > ```
> >
> > This will show that kheaders_data.tar.xz contains the symlinks, e.g. in my case:
> > ```
> > $ tar tvf ./kernel/kheaders_data.tar.xz | head
> > [...]
> > lrwxrwxrwx 0/0               0 2023-04-24 11:19
> > ./arch/arm64/include/asm/acenv.h ->
> > /mnt/sdb/linux/./arch/arm64/include/asm/acenv.h
> > ```
> >
> > However, the issue for me is that these symlinks are no longer useful
> > under sysfs when I am booting the kernel on the target arm64 machine,
> > because it is a different machine (arm64) than the machine (x86) where
> > I compiled this kernel.
> >
> > >
> > >
> > >
> > > I built the kernel inside a container,
> > > but I do not see any difference.
> > >
> > >
> > >
> > > We have some symbolic links in include/,
> > > but those reference relative paths.
> > > How building the kernel in a container
> > > makes a difference?
> >
> > Please see above; the container contains a forest of symlinks pointing
> > to the original source tree. Then `make` is executed in that container
> > directory.
> >
> > >
> > >
> > > masahiro@bc5c2c958b53:/tmp/foo$ tar Jxf kheaders_data.tar.xz
> > > masahiro@bc5c2c958b53:/tmp/foo$ find . -type l
> > > ./include/dt-bindings/clock/qcom,dispcc-sm8350.h
> > > ./include/dt-bindings/clock/qcom,dispcc-sm8150.h
> > > ./include/dt-bindings/input/linux-event-codes.h
> > > masahiro@bc5c2c958b53:/tmp/foo$ find . -type l | xargs ls -l
> > > lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
> > > ./include/dt-bindings/clock/qcom,dispcc-sm8150.h ->
> > > qcom,dispcc-sm8250.h
> > > lrwxrwxrwx 1 masahiro masahiro 20 Apr 23 11:47
> > > ./include/dt-bindings/clock/qcom,dispcc-sm8350.h ->
> > > qcom,dispcc-sm8250.h
> > > lrwxrwxrwx 1 masahiro masahiro 36 Apr 23 11:47
> > > ./include/dt-bindings/input/linux-event-codes.h ->
> > > ../../uapi/linux/input-event-codes.h
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > > ---
> > > >  kernel/gen_kheaders.sh | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh
> > > > index 1ef9a87511f5..70a75a9a7535 100755
> > > > --- a/kernel/gen_kheaders.sh
> > > > +++ b/kernel/gen_kheaders.sh
> > > > @@ -87,7 +87,7 @@ find $cpio_dir -type f -print0 |
> > > >  # pre-sorted, as --sort=name might not be available.
> > > >  find $cpio_dir -printf "./%P\n" | LC_ALL=C sort | \
> > > >      tar "${KBUILD_BUILD_TIMESTAMP:+--mtime=$KBUILD_BUILD_TIMESTAMP}" \
> > > > -    --owner=0 --group=0 --numeric-owner --no-recursion \
> > > > +    --owner=0 --group=0 --numeric-owner --no-recursion --dereference \
> > > >      -I $XZ -cf $tarfile -C $cpio_dir/ -T - > /dev/null
> > > >
> > > >  echo $headers_md5 > kernel/kheaders.md5
> > > > --
> > > > 2.40.0.634.g4ca3ef3211-goog
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Masahiro Yamada
> > >
> > > --
> > > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
> > >
>
> Gently nagging. Is there anything I need to fix or do to improve this
> patch so it may be accepted? Thank you!




I guess it is better to not apply this.

Presumably, the fix will not end with this single patch.



-- 
Best Regards
Masahiro Yamada