| Message ID | 20190523121803.21638-1-roberto.sassu@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | initramfs: add support for xattrs in the initial ram disk | expand |
Any opinion on this patch set? Thanks Roberto On 5/23/2019 2:18 PM, Roberto Sassu wrote: > This patch set aims at solving the following use case: appraise files from > the initial ram disk. To do that, IMA checks the signature/hash from the > security.ima xattr. Unfortunately, this use case cannot be implemented > currently, as the CPIO format does not support xattrs. > > This proposal consists in including file metadata as additional files named > METADATA!!!, for each file added to the ram disk. The CPIO parser in the > kernel recognizes these special files from the file name, and calls the > appropriate parser to add metadata to the previously extracted file. It has > been proposed to use bit 17:16 of the file mode as a way to recognize files > with metadata, but both the kernel and the cpio tool declare the file mode > as unsigned short. > > The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230, > https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in > separate files instead of a single file. Given that files with metadata > must immediately follow the files metadata will be added to, image > generators have to be modified in this version. > > The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that > all files have the same name. The file metadata are added to is always the > previous one, and the image generator in user space will make sure that > files are in the correct sequence. > > The difference with another proposal > (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be > included in an image without changing the image format. Files with metadata > will appear as regular files. It will be task of the parser in the kernel > to process them. > > This patch set extends the format of data defined in patch 9/15 of the last > proposal. It adds header version and type, so that new formats can be > defined and arbitrary metadata types can be processed. > > The changes introduced by this patch set don't cause any compatibility > issue: kernels without the metadata parser simply extract the special files > and don't process metadata; kernels with the metadata parser don't process > metadata if the special files are not included in the image. > > From the kernel space perspective, backporting this functionality to older > kernels should be very easy. It is sufficient to add two calls to the new > function do_process_metadata() in do_copy(), and to check the file name in > do_name(). From the user space perspective, unlike the previous version of > the patch set, it is required to modify the image generators in order to > include metadata as separate files. > > Changelog > > v3: > - include file metadata as separate files named METADATA!!! > - add the possibility to include in the ram disk arbitrary metadata types > > v2: > - replace ksys_lsetxattr() with kern_path() and vfs_setxattr() > (suggested by Jann Horn) > - replace ksys_open()/ksys_read()/ksys_close() with > filp_open()/kernel_read()/fput() > (suggested by Jann Horn) > - use path variable instead of name_buf in do_readxattrs() > - set last byte of str to 0 in do_readxattrs() > - call do_readxattrs() in do_name() before replacing an existing > .xattr-list > - pass pathname to do_setxattrs() > > v1: > - move xattr unmarshaling to CPIO parser > > > Mimi Zohar (1): > initramfs: add file metadata > > Roberto Sassu (2): > initramfs: read metadata from special file METADATA!!! > gen_init_cpio: add support for file metadata > > include/linux/initramfs.h | 21 ++++++ > init/initramfs.c | 137 +++++++++++++++++++++++++++++++++++++- > usr/Kconfig | 8 +++ > usr/Makefile | 4 +- > usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++-- > usr/gen_initramfs_list.sh | 10 ++- > 6 files changed, 305 insertions(+), 12 deletions(-) > create mode 100644 include/linux/initramfs.h >
On 6/3/19 4:31 AM, Roberto Sassu wrote: >> This patch set aims at solving the following use case: appraise files from >> the initial ram disk. To do that, IMA checks the signature/hash from the >> security.ima xattr. Unfortunately, this use case cannot be implemented >> currently, as the CPIO format does not support xattrs. >> >> This proposal consists in including file metadata as additional files named >> METADATA!!!, for each file added to the ram disk. The CPIO parser in the >> kernel recognizes these special files from the file name, and calls the >> appropriate parser to add metadata to the previously extracted file. It has >> been proposed to use bit 17:16 of the file mode as a way to recognize files >> with metadata, but both the kernel and the cpio tool declare the file mode >> as unsigned short. > > Any opinion on this patch set? > > Thanks > > Roberto Sorry, I've had the window open since you posted it but haven't gotten around to it. I'll try to build it later today. It does look interesting, and I have no objections to the basic approach. I should be able to add support to toybox cpio over a weekend once I've got the kernel doing it to test against. Rob
On 6/3/2019 8:32 PM, Rob Landley wrote: > On 6/3/19 4:31 AM, Roberto Sassu wrote: >>> This patch set aims at solving the following use case: appraise files from >>> the initial ram disk. To do that, IMA checks the signature/hash from the >>> security.ima xattr. Unfortunately, this use case cannot be implemented >>> currently, as the CPIO format does not support xattrs. >>> >>> This proposal consists in including file metadata as additional files named >>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the >>> kernel recognizes these special files from the file name, and calls the >>> appropriate parser to add metadata to the previously extracted file. It has >>> been proposed to use bit 17:16 of the file mode as a way to recognize files >>> with metadata, but both the kernel and the cpio tool declare the file mode >>> as unsigned short. >> >> Any opinion on this patch set? >> >> Thanks >> >> Roberto > > Sorry, I've had the window open since you posted it but haven't gotten around to > it. I'll try to build it later today. > > It does look interesting, and I have no objections to the basic approach. I > should be able to add support to toybox cpio over a weekend once I've got the > kernel doing it to test against. Ok. Let me give some instructions so that people can test this patch set. To add xattrs to the ram disk embedded in the kernel it is sufficient to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration. To add xattrs to the external ram disk, it is necessary to patch cpio: https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef (xattr-v1 branch) and dracut: https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b (digest-lists branch) The same modification can be done for mkinitramfs (add '-e xattr' to the cpio command line). To simplify the test, it would be sufficient to replace only the cpio binary and the dracut script with the modified versions. For dracut, the patch should be applied to the local dracut (after it has been renamed to dracut.sh). Then, run: dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk) Xattrs can be seen by stopping the boot process for example by adding rd.break to the kernel command line. Roberto
On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote: > On 6/3/2019 8:32 PM, Rob Landley wrote: > > On 6/3/19 4:31 AM, Roberto Sassu wrote: > >>> This patch set aims at solving the following use case: appraise files from > >>> the initial ram disk. To do that, IMA checks the signature/hash from the > >>> security.ima xattr. Unfortunately, this use case cannot be implemented > >>> currently, as the CPIO format does not support xattrs. > >>> > >>> This proposal consists in including file metadata as additional files named > >>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the > >>> kernel recognizes these special files from the file name, and calls the > >>> appropriate parser to add metadata to the previously extracted file. It has > >>> been proposed to use bit 17:16 of the file mode as a way to recognize files > >>> with metadata, but both the kernel and the cpio tool declare the file mode > >>> as unsigned short. > >> > >> Any opinion on this patch set? > >> > >> Thanks > >> > >> Roberto > > > > Sorry, I've had the window open since you posted it but haven't gotten around to > > it. I'll try to build it later today. > > > > It does look interesting, and I have no objections to the basic approach. I > > should be able to add support to toybox cpio over a weekend once I've got the > > kernel doing it to test against. > > Ok. > > Let me give some instructions so that people can test this patch set. > > To add xattrs to the ram disk embedded in the kernel it is sufficient > to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and > CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration. > > To add xattrs to the external ram disk, it is necessary to patch cpio: > > https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef > (xattr-v1 branch) > > and dracut: > > https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b > (digest-lists branch) > > The same modification can be done for mkinitramfs (add '-e xattr' to the > cpio command line). > > To simplify the test, it would be sufficient to replace only the cpio > binary and the dracut script with the modified versions. For dracut, the > patch should be applied to the local dracut (after it has been renamed > to dracut.sh). > > Then, run: > > dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk) > > Xattrs can be seen by stopping the boot process for example by adding > rd.break to the kernel command line. A simple way of testing, without needing any changes other than the kernel patches, is to save the dracut temporary directory by supplying "--keep" on the dracut command line, calling usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e xattr" option. If your filesystem already has and copied the security xattrs to the dracut temporary directory, the script, below, will include them in the initramfs file. Otherwise, you'll need to write the desired security xattrs on the files, using setfattr, in the temporary dracut directory, before creating the initramfs. Remember to make sure that the initramfs_list includes "getfattr", otherwise you'll need to wait until real root is mounted as /sysroot to see the security xattrs for the rootfs files. The following script has not been tested on a recent version of dracut. Some changes might be needed, as well as some code cleanup. #!/bin/bash initramfs_name=/boot/initramfs-`uname -r`.img initramfs_output_name=${initramfs_name/.img/.test.img} if [ $# -eq 1 ]; then initramfs_name=$1 fi if [ ! -f "$initramfs_name" ]; then echo "Usage; $0 <initramfs pathanem>" exit 1 fi tmp=$(dracut -H -f "$initramfs_name" --keep --noprelink --nostrip 2>&1) suffix=$(echo $tmp | cut -d ' ' -f 3 | cut -d '.' -f 2) tmpdir="/var/tmp/dracut.$suffix/initramfs" if [ ! -d "$tmpdir" ]; then echo "$tmpdir does not exist" exit 1 fi usr/gen_initramfs_list.sh ${tmpdir} > usr/initramfs_list usr/gen_init_cpio -e xattr usr/initramfs_list > usr/initramfs_data.cpio gzip usr/initramfs_data.cpio echo "Copying usr/initramfs_data.cpio to $initramfs_output_name" cp usr/initramfs_data.cpio.gz "$initramfs_output_name" Mimi
On Thu, 2019-05-23 at 14:18 +0200, Roberto Sassu wrote: > This patch set aims at solving the following use case: appraise files from > the initial ram disk. To do that, IMA checks the signature/hash from the > security.ima xattr. Unfortunately, this use case cannot be implemented > currently, as the CPIO format does not support xattrs. > > This proposal consists in including file metadata as additional files named > METADATA!!!, for each file added to the ram disk. The CPIO parser in the > kernel recognizes these special files from the file name, and calls the > appropriate parser to add metadata to the previously extracted file. It has > been proposed to use bit 17:16 of the file mode as a way to recognize files > with metadata, but both the kernel and the cpio tool declare the file mode > as unsigned short. Thanks, Roberto! Victor, Taras, Rob, Arvind, Peter, if you're good with this latest design, could we get some Reviewed-by, Acked-by, or Tested-by? thanks! Mimi
On 6/30/2019 6:39 PM, Mimi Zohar wrote: > On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote: >> On 6/3/2019 8:32 PM, Rob Landley wrote: >>> On 6/3/19 4:31 AM, Roberto Sassu wrote: >>>>> This patch set aims at solving the following use case: appraise files from >>>>> the initial ram disk. To do that, IMA checks the signature/hash from the >>>>> security.ima xattr. Unfortunately, this use case cannot be implemented >>>>> currently, as the CPIO format does not support xattrs. >>>>> >>>>> This proposal consists in including file metadata as additional files named >>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the >>>>> kernel recognizes these special files from the file name, and calls the >>>>> appropriate parser to add metadata to the previously extracted file. It has >>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files >>>>> with metadata, but both the kernel and the cpio tool declare the file mode >>>>> as unsigned short. >>>> >>>> Any opinion on this patch set? >>>> >>>> Thanks >>>> >>>> Roberto >>> >>> Sorry, I've had the window open since you posted it but haven't gotten around to >>> it. I'll try to build it later today. >>> >>> It does look interesting, and I have no objections to the basic approach. I >>> should be able to add support to toybox cpio over a weekend once I've got the >>> kernel doing it to test against. >> >> Ok. >> >> Let me give some instructions so that people can test this patch set. >> >> To add xattrs to the ram disk embedded in the kernel it is sufficient >> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and >> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration. >> >> To add xattrs to the external ram disk, it is necessary to patch cpio: >> >> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef >> (xattr-v1 branch) >> >> and dracut: >> >> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b >> (digest-lists branch) >> >> The same modification can be done for mkinitramfs (add '-e xattr' to the >> cpio command line). >> >> To simplify the test, it would be sufficient to replace only the cpio >> binary and the dracut script with the modified versions. For dracut, the >> patch should be applied to the local dracut (after it has been renamed >> to dracut.sh). >> >> Then, run: >> >> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk) >> >> Xattrs can be seen by stopping the boot process for example by adding >> rd.break to the kernel command line. > > A simple way of testing, without needing any changes other than the > kernel patches, is to save the dracut temporary directory by supplying > "--keep" on the dracut command line, calling > usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e > xattr" option. Alternatively, follow the instructions to create the embedded ram disk with xattrs, and use the existing external ram disk created with dracut to check if xattrs are created. Roberto
On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote: > On 6/30/2019 6:39 PM, Mimi Zohar wrote: > > On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote: > >> On 6/3/2019 8:32 PM, Rob Landley wrote: > >>> On 6/3/19 4:31 AM, Roberto Sassu wrote: > >>>>> This patch set aims at solving the following use case: appraise files from > >>>>> the initial ram disk. To do that, IMA checks the signature/hash from the > >>>>> security.ima xattr. Unfortunately, this use case cannot be implemented > >>>>> currently, as the CPIO format does not support xattrs. > >>>>> > >>>>> This proposal consists in including file metadata as additional files named > >>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the > >>>>> kernel recognizes these special files from the file name, and calls the > >>>>> appropriate parser to add metadata to the previously extracted file. It has > >>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files > >>>>> with metadata, but both the kernel and the cpio tool declare the file mode > >>>>> as unsigned short. > >>>> > >>>> Any opinion on this patch set? > >>>> > >>>> Thanks > >>>> > >>>> Roberto > >>> > >>> Sorry, I've had the window open since you posted it but haven't gotten around to > >>> it. I'll try to build it later today. > >>> > >>> It does look interesting, and I have no objections to the basic approach. I > >>> should be able to add support to toybox cpio over a weekend once I've got the > >>> kernel doing it to test against. > >> > >> Ok. > >> > >> Let me give some instructions so that people can test this patch set. > >> > >> To add xattrs to the ram disk embedded in the kernel it is sufficient > >> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and > >> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration. > >> > >> To add xattrs to the external ram disk, it is necessary to patch cpio: > >> > >> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef > >> (xattr-v1 branch) > >> > >> and dracut: > >> > >> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b > >> (digest-lists branch) > >> > >> The same modification can be done for mkinitramfs (add '-e xattr' to the > >> cpio command line). > >> > >> To simplify the test, it would be sufficient to replace only the cpio > >> binary and the dracut script with the modified versions. For dracut, the > >> patch should be applied to the local dracut (after it has been renamed > >> to dracut.sh). > >> > >> Then, run: > >> > >> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk) > >> > >> Xattrs can be seen by stopping the boot process for example by adding > >> rd.break to the kernel command line. > > > > A simple way of testing, without needing any changes other than the > > kernel patches, is to save the dracut temporary directory by supplying > > "--keep" on the dracut command line, calling > > usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e > > xattr" option. > > Alternatively, follow the instructions to create the embedded ram disk > with xattrs, and use the existing external ram disk created with dracut > to check if xattrs are created. True, but this alternative is for those who normally use dracut to create an initramfs, but don't want to update cpio or dracut. Mimi
Rob, Peter, Arvind, did you have the chance to have a look at this version of the patch set? Thanks Roberto On 7/1/2019 4:31 PM, Mimi Zohar wrote: > On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote: >> On 6/30/2019 6:39 PM, Mimi Zohar wrote: >>> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote: >>>> On 6/3/2019 8:32 PM, Rob Landley wrote: >>>>> On 6/3/19 4:31 AM, Roberto Sassu wrote: >>>>>>> This patch set aims at solving the following use case: appraise files from >>>>>>> the initial ram disk. To do that, IMA checks the signature/hash from the >>>>>>> security.ima xattr. Unfortunately, this use case cannot be implemented >>>>>>> currently, as the CPIO format does not support xattrs. >>>>>>> >>>>>>> This proposal consists in including file metadata as additional files named >>>>>>> METADATA!!!, for each file added to the ram disk. The CPIO parser in the >>>>>>> kernel recognizes these special files from the file name, and calls the >>>>>>> appropriate parser to add metadata to the previously extracted file. It has >>>>>>> been proposed to use bit 17:16 of the file mode as a way to recognize files >>>>>>> with metadata, but both the kernel and the cpio tool declare the file mode >>>>>>> as unsigned short. >>>>>> >>>>>> Any opinion on this patch set? >>>>>> >>>>>> Thanks >>>>>> >>>>>> Roberto >>>>> >>>>> Sorry, I've had the window open since you posted it but haven't gotten around to >>>>> it. I'll try to build it later today. >>>>> >>>>> It does look interesting, and I have no objections to the basic approach. I >>>>> should be able to add support to toybox cpio over a weekend once I've got the >>>>> kernel doing it to test against. >>>> >>>> Ok. >>>> >>>> Let me give some instructions so that people can test this patch set. >>>> >>>> To add xattrs to the ram disk embedded in the kernel it is sufficient >>>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and >>>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel configuration. >>>> >>>> To add xattrs to the external ram disk, it is necessary to patch cpio: >>>> >>>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef >>>> (xattr-v1 branch) >>>> >>>> and dracut: >>>> >>>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b >>>> (digest-lists branch) >>>> >>>> The same modification can be done for mkinitramfs (add '-e xattr' to the >>>> cpio command line). >>>> >>>> To simplify the test, it would be sufficient to replace only the cpio >>>> binary and the dracut script with the modified versions. For dracut, the >>>> patch should be applied to the local dracut (after it has been renamed >>>> to dracut.sh). >>>> >>>> Then, run: >>>> >>>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram disk) >>>> >>>> Xattrs can be seen by stopping the boot process for example by adding >>>> rd.break to the kernel command line. >>> >>> A simple way of testing, without needing any changes other than the >>> kernel patches, is to save the dracut temporary directory by supplying >>> "--keep" on the dracut command line, calling >>> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e >>> xattr" option. >> >> Alternatively, follow the instructions to create the embedded ram disk >> with xattrs, and use the existing external ram disk created with dracut >> to check if xattrs are created. > > True, but this alternative is for those who normally use dracut to > create an initramfs, but don't want to update cpio or dracut. > > Mimi >
Is there anything I didn't address in this patch set, that is delaying the review? I would appreciate if you can give me a feedback, positive or negative. Thanks a lot! Roberto On 7/15/2019 6:54 PM, Roberto Sassu wrote: > Rob, Peter, Arvind, did you have the chance to have a look at this > version of the patch set? > > Thanks > > Roberto > > > On 7/1/2019 4:31 PM, Mimi Zohar wrote: >> On Mon, 2019-07-01 at 16:42 +0300, Roberto Sassu wrote: >>> On 6/30/2019 6:39 PM, Mimi Zohar wrote: >>>> On Wed, 2019-06-26 at 10:15 +0200, Roberto Sassu wrote: >>>>> On 6/3/2019 8:32 PM, Rob Landley wrote: >>>>>> On 6/3/19 4:31 AM, Roberto Sassu wrote: >>>>>>>> This patch set aims at solving the following use case: appraise >>>>>>>> files from >>>>>>>> the initial ram disk. To do that, IMA checks the signature/hash >>>>>>>> from the >>>>>>>> security.ima xattr. Unfortunately, this use case cannot be >>>>>>>> implemented >>>>>>>> currently, as the CPIO format does not support xattrs. >>>>>>>> >>>>>>>> This proposal consists in including file metadata as additional >>>>>>>> files named >>>>>>>> METADATA!!!, for each file added to the ram disk. The CPIO >>>>>>>> parser in the >>>>>>>> kernel recognizes these special files from the file name, and >>>>>>>> calls the >>>>>>>> appropriate parser to add metadata to the previously extracted >>>>>>>> file. It has >>>>>>>> been proposed to use bit 17:16 of the file mode as a way to >>>>>>>> recognize files >>>>>>>> with metadata, but both the kernel and the cpio tool declare the >>>>>>>> file mode >>>>>>>> as unsigned short. >>>>>>> >>>>>>> Any opinion on this patch set? >>>>>>> >>>>>>> Thanks >>>>>>> >>>>>>> Roberto >>>>>> >>>>>> Sorry, I've had the window open since you posted it but haven't >>>>>> gotten around to >>>>>> it. I'll try to build it later today. >>>>>> >>>>>> It does look interesting, and I have no objections to the basic >>>>>> approach. I >>>>>> should be able to add support to toybox cpio over a weekend once >>>>>> I've got the >>>>>> kernel doing it to test against. >>>>> >>>>> Ok. >>>>> >>>>> Let me give some instructions so that people can test this patch set. >>>>> >>>>> To add xattrs to the ram disk embedded in the kernel it is sufficient >>>>> to set CONFIG_INITRAMFS_FILE_METADATA="xattr" and >>>>> CONFIG_INITRAMFS_SOURCE="<file with xattr>" in the kernel >>>>> configuration. >>>>> >>>>> To add xattrs to the external ram disk, it is necessary to patch cpio: >>>>> >>>>> https://github.com/euleros/cpio/commit/531cabc88e9ecdc3231fad6e4856869baa9a91ef >>>>> >>>>> (xattr-v1 branch) >>>>> >>>>> and dracut: >>>>> >>>>> https://github.com/euleros/dracut/commit/a2dee56ea80495c2c1871bc73186f7b00dc8bf3b >>>>> >>>>> (digest-lists branch) >>>>> >>>>> The same modification can be done for mkinitramfs (add '-e xattr' >>>>> to the >>>>> cpio command line). >>>>> >>>>> To simplify the test, it would be sufficient to replace only the cpio >>>>> binary and the dracut script with the modified versions. For >>>>> dracut, the >>>>> patch should be applied to the local dracut (after it has been renamed >>>>> to dracut.sh). >>>>> >>>>> Then, run: >>>>> >>>>> dracut -e xattr -I <file with xattr> (add -f to overwrite the ram >>>>> disk) >>>>> >>>>> Xattrs can be seen by stopping the boot process for example by adding >>>>> rd.break to the kernel command line. >>>> >>>> A simple way of testing, without needing any changes other than the >>>> kernel patches, is to save the dracut temporary directory by supplying >>>> "--keep" on the dracut command line, calling >>>> usr/gen_initramfs_list.sh, followed by usr/gen_init_cpio with the "-e >>>> xattr" option. >>> >>> Alternatively, follow the instructions to create the embedded ram disk >>> with xattrs, and use the existing external ram disk created with dracut >>> to check if xattrs are created. >> >> True, but this alternative is for those who normally use dracut to >> create an initramfs, but don't want to update cpio or dracut. >> >> Mimi >> >
Dear Roberto, Cc: Yamada-san, linux-kbuild On Mi, Jul 24, 2019 at 05:34:53 +0200, Roberto Sassu wrote: > Is there anything I didn't address in this patch set, that is delaying > the review? I would appreciate if you can give me a feedback, positive > or negative. > > Thanks a lot! > > Roberto Some of our users have recently asked for this patch series. Could you please feedback if this is the latest revision available or maybe there is a newer one developed and potentially not shared on LKML? Appreciate your time. Thanks and Best Regards, Eugeniu
> From: Eugeniu Rosca [mailto:erosca@de.adit-jv.com] > Sent: Thursday, June 9, 2022 12:26 PM > Dear Roberto, > Cc: Yamada-san, linux-kbuild > > On Mi, Jul 24, 2019 at 05:34:53 +0200, Roberto Sassu wrote: > > Is there anything I didn't address in this patch set, that is delaying > > the review? I would appreciate if you can give me a feedback, positive > > or negative. > > > > Thanks a lot! > > > > Roberto > > Some of our users have recently asked for this patch series. Hello thanks for your interest in this patch set. > Could you please feedback if this is the latest revision available or > maybe there is a newer one developed and potentially not shared on LKML? Yes, it is the latest revision available. There might have been few fixes in the final code. You may want to have a look at: https://github.com/openeuler-mirror/kernel/commit/888460f17775b62f77e33e774e6673587c61cabd https://github.com/openeuler-mirror/kernel/commit/4adaeecd5d23cc75ffd1883d9b677bbd67c535d1 https://github.com/openeuler-mirror/kernel/commit/59db8952e91c2ac443bccdcacfd37ae94c49a259 and: https://gitee.com/src-openeuler/cpio/blob/master/add-option-to-add-metadata-in-copy-out-mode.patch https://gitee.com/src-openeuler/cpio/blob/master/Fix-use-after-free-and-return-appropriate-error.patch Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He
Hello Roberto, On Do, Jun 09, 2022 at 11:05:45 +0000, Roberto Sassu wrote: > > From: Eugeniu Rosca [mailto:erosca@de.adit-jv.com] > > Sent: Thursday, June 9, 2022 12:26 PM > > Dear Roberto, > > Cc: Yamada-san, linux-kbuild > > > > On Mi, Jul 24, 2019 at 05:34:53 +0200, Roberto Sassu wrote: > > > Is there anything I didn't address in this patch set, that is delaying > > > the review? I would appreciate if you can give me a feedback, positive > > > or negative. > > > > > > Thanks a lot! > > > > > > Roberto > > > > Some of our users have recently asked for this patch series. > > Hello > > thanks for your interest in this patch set. > > > Could you please feedback if this is the latest revision available or > > maybe there is a newer one developed and potentially not shared on LKML? > > Yes, it is the latest revision available. There might have been few > fixes in the final code. You may want to have a look at: Many thanks for the links to the updated patch revisions. It looks like the new versions added a couple of bugfixes and refinements. With more users now using this feature, do you think there is a higher chance for upstreaming, compared to 2019 (original submission date)? Best Regards, Eugeniu
> From: Eugeniu Rosca [mailto:erosca@de.adit-jv.com] > Sent: Friday, June 10, 2022 5:34 PM > Hello Roberto, > > On Do, Jun 09, 2022 at 11:05:45 +0000, Roberto Sassu wrote: > > > From: Eugeniu Rosca [mailto:erosca@de.adit-jv.com] > > > Sent: Thursday, June 9, 2022 12:26 PM > > > Dear Roberto, > > > Cc: Yamada-san, linux-kbuild > > > > > > On Mi, Jul 24, 2019 at 05:34:53 +0200, Roberto Sassu wrote: > > > > Is there anything I didn't address in this patch set, that is delaying > > > > the review? I would appreciate if you can give me a feedback, positive > > > > or negative. > > > > > > > > Thanks a lot! > > > > > > > > Roberto > > > > > > Some of our users have recently asked for this patch series. > > > > Hello > > > > thanks for your interest in this patch set. > > > > > Could you please feedback if this is the latest revision available or > > > maybe there is a newer one developed and potentially not shared on LKML? > > > > Yes, it is the latest revision available. There might have been few > > fixes in the final code. You may want to have a look at: > > Many thanks for the links to the updated patch revisions. It looks > like the new versions added a couple of bugfixes and refinements. > > With more users now using this feature, do you think there is a higher > chance for upstreaming, compared to 2019 (original submission date)? Hello Eugeniu I would be happy to address the remaining concerns, or take more suggestions, and then develop a new version of the patch set. Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He
Hello Roberto, On Fr, Jun 10, 2022 at 03:38:24 +0000, Roberto Sassu wrote: > I would be happy to address the remaining concerns, or take more > suggestions, and then develop a new version of the patch set. I face a number of conflicts when I try to rebase the latest openEuler commits against vanilla master (v5.19-rc2). Do you think it is possible to submit the rebased version to ML? In addition, I can also see some open/unresolved points from Mimi [*]. Did you by chance find some mutual agreement offline or do you think they would still potentially need some attention? Maybe we can resume the discussion once you submit the rebased series? Many thanks and looking forward to it. [*] Potentially comments which deserve a reply/clarification/resolution https://lore.kernel.org/lkml/1561985652.4049.24.camel@linux.ibm.com/#t https://lore.kernel.org/lkml/1561908456.3985.23.camel@linux.ibm.com/ BR, Eugeniu.
From: Roberto Sassu <roberto.sassu@huawei.com> Date: Thu, 23 May 2019 14:18:00 +0200 > This patch set aims at solving the following use case: appraise files from > the initial ram disk. To do that, IMA checks the signature/hash from the Hi, is this[0] relatable somehow? > security.ima xattr. Unfortunately, this use case cannot be implemented > currently, as the CPIO format does not support xattrs. > > This proposal consists in including file metadata as additional files named > METADATA!!!, for each file added to the ram disk. The CPIO parser in the > kernel recognizes these special files from the file name, and calls the > appropriate parser to add metadata to the previously extracted file. It has > been proposed to use bit 17:16 of the file mode as a way to recognize files > with metadata, but both the kernel and the cpio tool declare the file mode > as unsigned short. > > The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230, > https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in > separate files instead of a single file. Given that files with metadata > must immediately follow the files metadata will be added to, image > generators have to be modified in this version. > > The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that > all files have the same name. The file metadata are added to is always the > previous one, and the image generator in user space will make sure that > files are in the correct sequence. > > The difference with another proposal > (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be > included in an image without changing the image format. Files with metadata > will appear as regular files. It will be task of the parser in the kernel > to process them. > > This patch set extends the format of data defined in patch 9/15 of the last > proposal. It adds header version and type, so that new formats can be > defined and arbitrary metadata types can be processed. > > The changes introduced by this patch set don't cause any compatibility > issue: kernels without the metadata parser simply extract the special files > and don't process metadata; kernels with the metadata parser don't process > metadata if the special files are not included in the image. > > >>From the kernel space perspective, backporting this functionality to older > kernels should be very easy. It is sufficient to add two calls to the new > function do_process_metadata() in do_copy(), and to check the file name in > do_name(). From the user space perspective, unlike the previous version of > the patch set, it is required to modify the image generators in order to > include metadata as separate files. > > Changelog > > v3: > - include file metadata as separate files named METADATA!!! > - add the possibility to include in the ram disk arbitrary metadata types > > v2: > - replace ksys_lsetxattr() with kern_path() and vfs_setxattr() > (suggested by Jann Horn) > - replace ksys_open()/ksys_read()/ksys_close() with > filp_open()/kernel_read()/fput() > (suggested by Jann Horn) > - use path variable instead of name_buf in do_readxattrs() > - set last byte of str to 0 in do_readxattrs() > - call do_readxattrs() in do_name() before replacing an existing > .xattr-list > - pass pathname to do_setxattrs() > > v1: > - move xattr unmarshaling to CPIO parser > > > Mimi Zohar (1): > initramfs: add file metadata > > Roberto Sassu (2): > initramfs: read metadata from special file METADATA!!! > gen_init_cpio: add support for file metadata > > include/linux/initramfs.h | 21 ++++++ > init/initramfs.c | 137 +++++++++++++++++++++++++++++++++++++- > usr/Kconfig | 8 +++ > usr/Makefile | 4 +- > usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++-- > usr/gen_initramfs_list.sh | 10 ++- > 6 files changed, 305 insertions(+), 12 deletions(-) > create mode 100644 include/linux/initramfs.h > > -- > 2.17.1 [0] https://lore.kernel.org/all/20210702233727.21301-1-alobakin@pm.me Thanks, Olek
> From: Alexander Lobakin [mailto:alexandr.lobakin@intel.com] > Sent: Wednesday, June 15, 2022 5:51 PM > From: Roberto Sassu <roberto.sassu@huawei.com> > Date: Thu, 23 May 2019 14:18:00 +0200 > > > This patch set aims at solving the following use case: appraise files from > > the initial ram disk. To do that, IMA checks the signature/hash from the > > Hi, > is this[0] relatable somehow? Hi Alexander seems a separate problem. For that, we opted for having a dedicated kernel option: https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da Thanks Roberto HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Yang Xi, Li He > > security.ima xattr. Unfortunately, this use case cannot be implemented > > currently, as the CPIO format does not support xattrs. > > > > This proposal consists in including file metadata as additional files named > > METADATA!!!, for each file added to the ram disk. The CPIO parser in the > > kernel recognizes these special files from the file name, and calls the > > appropriate parser to add metadata to the previously extracted file. It has > > been proposed to use bit 17:16 of the file mode as a way to recognize files > > with metadata, but both the kernel and the cpio tool declare the file mode > > as unsigned short. > > > > The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230, > > https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in > > separate files instead of a single file. Given that files with metadata > > must immediately follow the files metadata will be added to, image > > generators have to be modified in this version. > > > > The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that > > all files have the same name. The file metadata are added to is always the > > previous one, and the image generator in user space will make sure that > > files are in the correct sequence. > > > > The difference with another proposal > > (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be > > included in an image without changing the image format. Files with metadata > > will appear as regular files. It will be task of the parser in the kernel > > to process them. > > > > This patch set extends the format of data defined in patch 9/15 of the last > > proposal. It adds header version and type, so that new formats can be > > defined and arbitrary metadata types can be processed. > > > > The changes introduced by this patch set don't cause any compatibility > > issue: kernels without the metadata parser simply extract the special files > > and don't process metadata; kernels with the metadata parser don't process > > metadata if the special files are not included in the image. > > > > >>From the kernel space perspective, backporting this functionality to older > > kernels should be very easy. It is sufficient to add two calls to the new > > function do_process_metadata() in do_copy(), and to check the file name in > > do_name(). From the user space perspective, unlike the previous version of > > the patch set, it is required to modify the image generators in order to > > include metadata as separate files. > > > > Changelog > > > > v3: > > - include file metadata as separate files named METADATA!!! > > - add the possibility to include in the ram disk arbitrary metadata types > > > > v2: > > - replace ksys_lsetxattr() with kern_path() and vfs_setxattr() > > (suggested by Jann Horn) > > - replace ksys_open()/ksys_read()/ksys_close() with > > filp_open()/kernel_read()/fput() > > (suggested by Jann Horn) > > - use path variable instead of name_buf in do_readxattrs() > > - set last byte of str to 0 in do_readxattrs() > > - call do_readxattrs() in do_name() before replacing an existing > > .xattr-list > > - pass pathname to do_setxattrs() > > > > v1: > > - move xattr unmarshaling to CPIO parser > > > > > > Mimi Zohar (1): > > initramfs: add file metadata > > > > Roberto Sassu (2): > > initramfs: read metadata from special file METADATA!!! > > gen_init_cpio: add support for file metadata > > > > include/linux/initramfs.h | 21 ++++++ > > init/initramfs.c | 137 +++++++++++++++++++++++++++++++++++++- > > usr/Kconfig | 8 +++ > > usr/Makefile | 4 +- > > usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++-- > > usr/gen_initramfs_list.sh | 10 ++- > > 6 files changed, 305 insertions(+), 12 deletions(-) > > create mode 100644 include/linux/initramfs.h > > > > -- > > 2.17.1 > > [0] https://lore.kernel.org/all/20210702233727.21301-1-alobakin@pm.me > > Thanks, > Olek
Dear Yamada-san, On Do, Mai 23, 2019 at 02:18:00 +0200, Roberto Sassu wrote: > This patch set aims at solving the following use case: appraise files from > the initial ram disk. To do that, IMA checks the signature/hash from the > security.ima xattr. Unfortunately, this use case cannot be implemented > currently, as the CPIO format does not support xattrs. > > This proposal consists in including file metadata as additional files named > METADATA!!!, for each file added to the ram disk. The CPIO parser in the > kernel recognizes these special files from the file name, and calls the > appropriate parser to add metadata to the previously extracted file. It has > been proposed to use bit 17:16 of the file mode as a way to recognize files > with metadata, but both the kernel and the cpio tool declare the file mode > as unsigned short. > > The difference from v2, v3 (https://lkml.org/lkml/2019/5/9/230, > https://lkml.org/lkml/2019/5/17/466) is that file metadata are stored in > separate files instead of a single file. Given that files with metadata > must immediately follow the files metadata will be added to, image > generators have to be modified in this version. > > The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that > all files have the same name. The file metadata are added to is always the > previous one, and the image generator in user space will make sure that > files are in the correct sequence. > > The difference with another proposal > (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be > included in an image without changing the image format. Files with metadata > will appear as regular files. It will be task of the parser in the kernel > to process them. > > This patch set extends the format of data defined in patch 9/15 of the last > proposal. It adds header version and type, so that new formats can be > defined and arbitrary metadata types can be processed. > > The changes introduced by this patch set don't cause any compatibility > issue: kernels without the metadata parser simply extract the special files > and don't process metadata; kernels with the metadata parser don't process > metadata if the special files are not included in the image. > > >From the kernel space perspective, backporting this functionality to older > kernels should be very easy. It is sufficient to add two calls to the new > function do_process_metadata() in do_copy(), and to check the file name in > do_name(). From the user space perspective, unlike the previous version of > the patch set, it is required to modify the image generators in order to > include metadata as separate files. Since this patch series most likely falls under your jurisdiction and also given your recent commits [*] in the same area, I am curious if there are any early signs which would prevent your final acceptance and would potentially result in a no-Go? Can we have an early confirmation that, upon rebasing and handling of all the review comments, you would be willing to accept the patches? [*] Most recent commits touching usr/gen_initramfs.sh https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7168965ec7b10b8a2c7dea1f82f1ebadf44d64ba https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65e00e04e5aea34b256814cfa21b32e3b94a2402 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=469e87e89fd61de804bd29f6dd0380a399b567a7 Thanks, Eugeniu.
On 15/06/2022 10:27, Eugeniu Rosca wrote: > Hello Roberto, > > On Fr, Jun 10, 2022 at 03:38:24 +0000, Roberto Sassu wrote: >> I would be happy to address the remaining concerns, or take more >> suggestions, and then develop a new version of the patch set. > I face a number of conflicts when I try to rebase the latest openEuler > commits against vanilla master (v5.19-rc2). Do you think it is possible > to submit the rebased version to ML? > > In addition, I can also see some open/unresolved points from Mimi [*]. > Did you by chance find some mutual agreement offline or do you think > they would still potentially need some attention? > > Maybe we can resume the discussion once you submit the rebased series? > > Many thanks and looking forward to it. > > [*] Potentially comments which deserve a reply/clarification/resolution > > https://lore.kernel.org/lkml/1561985652.4049.24.camel@linux.ibm.com/#t > https://lore.kernel.org/lkml/1561908456.3985.23.camel@linux.ibm.com/ > > BR, Eugeniu. > Hello, I have been testing these patches and do not see the xattr information when trying to retrieve it within the initramfs, do you have an example of how you tested this originally? So far I have set the xattr in the rootfs before creating the cpio file like this: $ setfattr -n user.comment -v "this is a comment" test.txt If I access the data here it works: $ getfattr test.txt # file: test.txt user.comment Then I package it and try to verify it with this command: $getfattr /test.txt Which returns to the command line without the data. I believe the cpio is working because I see the file /METADATA\!\!\! in the target root filesystem, which shows the following when viewed with cat -e: 00000028^A^Auser.comment^@this is a comment This matches the data I fed in at the start, so I believe the data is being transferred correctly but I am accessioning it with the wrong tools. Thank you for any help. Best regards, Jim
> From: Jim Baxter [mailto:jim_baxter@mentor.com] > Sent: Monday, July 18, 2022 6:36 PM > On 15/06/2022 10:27, Eugeniu Rosca wrote: > > Hello Roberto, > > > > On Fr, Jun 10, 2022 at 03:38:24 +0000, Roberto Sassu wrote: > >> I would be happy to address the remaining concerns, or take more > >> suggestions, and then develop a new version of the patch set. > > I face a number of conflicts when I try to rebase the latest openEuler > > commits against vanilla master (v5.19-rc2). Do you think it is possible > > to submit the rebased version to ML? > > > > In addition, I can also see some open/unresolved points from Mimi [*]. > > Did you by chance find some mutual agreement offline or do you think > > they would still potentially need some attention? > > > > Maybe we can resume the discussion once you submit the rebased series? > > > > Many thanks and looking forward to it. > > > > [*] Potentially comments which deserve a reply/clarification/resolution > > > > https://lore.kernel.org/lkml/1561985652.4049.24.camel@linux.ibm.com/#t > > https://lore.kernel.org/lkml/1561908456.3985.23.camel@linux.ibm.com/ > > > > BR, Eugeniu. > > > > > Hello, > > I have been testing these patches and do not see the xattr information when > trying to retrieve it within the initramfs, do you have an example of how > you tested this originally? Hi Jim, all apologies, I didn't find yet the time to look at this. Uhm, I guess this could be solved with: https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da and adding initramtmpfs to the kernel command line. You are probably using ramfs, which does not have xattr support. > So far I have set the xattr in the rootfs before creating the cpio file like this: > $ setfattr -n user.comment -v "this is a comment" test.txt > If I access the data here it works: > $ getfattr test.txt > # file: test.txt > user.comment > > > Then I package it and try to verify it with this command: > $getfattr /test.txt I assume you try to pack/unpack, right? If I remember correctly I only implemented the pack part. Unpacking is done by the kernel (but you are right, it should be done by user space too). > Which returns to the command line without the data. > > > > I believe the cpio is working because I see the file /METADATA\!\!\! in > the target root filesystem, which shows the following when viewed with cat -e: > 00000028^A^Auser.comment^@this is a comment > > This matches the data I fed in at the start, so I believe the data is being > transferred correctly but I am accessioning it with the wrong tools. Yes, xattrs are marshalled in the METADATA!!! file, one per regular file xattrs are applied to. Xattrs are applied to the previous regular file. That file name was preferred to adding a suffix to the file, to avoid reaching the filename size limit. Roberto
Best regards, *Jim Baxter* Siemens Digital Industries Software Automotive Business Unit DI SW STS ABU UK Tel.: +44 (161) 926-1656 mailto:jim.baxter@siemens.com <mailto:jim.baxter@siemens.com> sw.siemens.com <https://sw.siemens.com/> On 18/07/2022 17:49, Roberto Sassu wrote: >> From: Jim Baxter [mailto:jim_baxter@mentor.com] >> Sent: Monday, July 18, 2022 6:36 PM >> >> >> Hello, >> >> I have been testing these patches and do not see the xattr information when >> trying to retrieve it within the initramfs, do you have an example of how >> you tested this originally? > > Hi Jim, all > > apologies, I didn't find yet the time to look at this. Hello Roberto, Thank you for your response, I can wait until you have looked at the patches, I asked the question to make sure it was not something wrong in my configuration. > > Uhm, I guess this could be solved with: > > https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da > > and adding initramtmpfs to the kernel command line. You are > probably using ramfs, which does not have xattr support. > Thank you, I have tested that patch but the problem remained. Here is my command line, I wonder if there is something wrong. Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 initrd=0x500000000 rootwait I also found that root is always mounted as rootfs in my initramfs system which I understood to be tmpfs, is that incorrect? sh-3.2# mount none on / type rootfs (rw) >> So far I have set the xattr in the rootfs before creating the cpio file like this: >> $ setfattr -n user.comment -v "this is a comment" test.txt >> If I access the data here it works: >> $ getfattr test.txt >> # file: test.txt >> user.comment >> >> >> Then I package it and try to verify it with this command: >> $getfattr /test.txt > > I assume you try to pack/unpack, right? If I remember correctly > I only implemented the pack part. Unpacking is done by the kernel > (but you are right, it should be done by user space too). > I modified the file before packing. To pack I use the following commands: $ ./usr/gen_initramfs.sh -l initramfs.list -e xattr ../rootfs > initramfs.cpio $ gzip initramfs.cpio $ mkimage -A arm64 -O linux -T ramdisk -d initramfs.cpio.gz uRamdisk The kernel is loaded using: booti ${kernaddr} ${initramaddr} ${dtbaddr} >> Which returns to the command line without the data. >> >> >> >> I believe the cpio is working because I see the file /METADATA\!\!\! in >> the target root filesystem, which shows the following when viewed with cat -e: >> 00000028^A^Auser.comment^@this is a comment >> >> This matches the data I fed in at the start, so I believe the data is being >> transferred correctly but I am accessioning it with the wrong tools. > > Yes, xattrs are marshalled in the METADATA!!! file, one per regular file > xattrs are applied to. Xattrs are applied to the previous regular file. > That file name was preferred to adding a suffix to the file, to avoid > reaching the filename size limit. > > Roberto Best regards, Jim
> From: Jim Baxter [mailto:jim_baxter@mentor.com] > Sent: Monday, July 18, 2022 8:08 PM > > > > Best regards, > > *Jim Baxter* > > Siemens Digital Industries Software > Automotive Business Unit > DI SW STS ABU > UK > Tel.: +44 (161) 926-1656 > mailto:jim.baxter@siemens.com <mailto:jim.baxter@siemens.com> > sw.siemens.com <https://sw.siemens.com/> > > On 18/07/2022 17:49, Roberto Sassu wrote: > >> From: Jim Baxter [mailto:jim_baxter@mentor.com] > >> Sent: Monday, July 18, 2022 6:36 PM > >> > >> > >> Hello, > >> > >> I have been testing these patches and do not see the xattr information when > >> trying to retrieve it within the initramfs, do you have an example of how > >> you tested this originally? > > > > Hi Jim, all > > > > apologies, I didn't find yet the time to look at this. > > Hello Roberto, > > Thank you for your response, I can wait until you have looked at the patches, > I asked the question to make sure it was not something wrong in my > configuration. > > > > > Uhm, I guess this could be solved with: > > > > https://github.com/openeuler- > mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da > > > > and adding initramtmpfs to the kernel command line. You are > > probably using ramfs, which does not have xattr support. > > > > > Thank you, I have tested that patch but the problem remained. Here is my > command line, I wonder if there is something wrong. > > Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 > initrd=0x500000000 rootwait It is just initramtmpfs, without rootfstype=. Roberto
On 7/18/22 11:49, Roberto Sassu wrote: > Uhm, I guess this could be solved with: > > https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da > > and adding initramtmpfs to the kernel command line. It's initmpfs. You can argue about whether it should have two t's (I was consistent naming it in the patch series adding it), but ramfs and tmpfs are two different things and saying "initramtmpfs" is like saying "mount -t ext4btrfs". > You are probably using ramfs, which does not have xattr support. Do not specify root= in your kernel command line. If you specify root= you're saying "switch off of initramfs to a different root filesystem", so it doesn't make the overmounted filesystem tmpfs because you told it you wouldn't be using it. (The decision of what to mount has to be made before it examines the cpio.gz contents, so root= is used to signal "we are not keeping this initramfs" because that's literally what root= means. Your root filesystem is not initramfs, it is instead this thing to be mounted over initramfs.) You can tell which you're using via /proc/mounts having a line: rootfs / rootfs rw,size=121832k,nr_inodes=30458 0 0 If it's got the size= then it's tmpfs: ramfs basically doesn't have bounds checking and "cat /dev/null > filename" on ramfs will lock your system solid due to unpinnable memory exhaustion. If you don't have a "rootfs" line at ALL then root= was used to overmount and part of the gratuitously magic behavior of root= is it hides the rootfs line from /proc/mounts even though the filesystem is actually still there, which is not something it does for ANY OTHER OVERMOUNT: $ mkdir sub $ mount -t proc proc sub $ mount -t ramfs sub sub $ grep sub /proc/mounts proc /sub proc rw,relatime 0 0 sub /sub ramfs rw,relatime 0 0 I've never understood why they added that gratuitous special case to hide how the system actually works, but it's a land mine you have to be told about after you've stepped on it in order to understand what's going on. Part of the reason people think initramfs is so "magic" when PID 1 isn't, we don't HIDE the fact that PID 1 is always there but we hide the fact initramfs is... Rob
On 7/18/22 13:08, Jim Baxter wrote: > > > Best regards, > > *Jim Baxter* > > Siemens Digital Industries Software > Automotive Business Unit > DI SW STS ABU > UK > Tel.: +44 (161) 926-1656 > mailto:jim.baxter@siemens.com <mailto:jim.baxter@siemens.com> > sw.siemens.com <https://sw.siemens.com/> > > On 18/07/2022 17:49, Roberto Sassu wrote: >>> From: Jim Baxter [mailto:jim_baxter@mentor.com] >>> Sent: Monday, July 18, 2022 6:36 PM >>> >>> >>> Hello, >>> >>> I have been testing these patches and do not see the xattr information when >>> trying to retrieve it within the initramfs, do you have an example of how >>> you tested this originally? >> >> Hi Jim, all >> >> apologies, I didn't find yet the time to look at this. > > Hello Roberto, > > Thank you for your response, I can wait until you have looked at the patches, > I asked the question to make sure it was not something wrong in my > configuration. > >> >> Uhm, I guess this could be solved with: >> >> https://github.com/openeuler-mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da >> >> and adding initramtmpfs to the kernel command line. You are >> probably using ramfs, which does not have xattr support. >> > > > Thank you, I have tested that patch but the problem remained. Here is my > command line, I wonder if there is something wrong. > > Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 initrd=0x500000000 rootwait /dev/ram0 is a block device. Trying to provide it to tmpfs is like trying to say: mount -t proc /dev/sda1 /proc There's nowhere for the block device to GO because it's not a block backed filesystem. There's four types of filesystem: block back, pipe backed, ram backed, and synthetic. - Only block backed filesystems take a block device argument. Block backed filesystems require two drivers: one to handle I/O to the block device and one to interpret the filesystem format with the block device. You do not "format" any other kind of filesystem. (There's no mkfs.nfs or mkfs.proc: it doesn't work that way.) - Pipe backed ones include network filesystems (nfs, samba), FUSE filesystems, or hybrid weirdness like https://wiki.qemu.org/Documentation/9psetup . These drivers talk a protocol over a pipe (or network socket, or char device, or...) to a server at the far end that serves up the filesystem contents. Usually their source argument is a server address plus filesystem identification plus login credentials. Often they have a wrapper program that assembles this argument for you. - Ram backed filesystems (ramfs, tmpfs) treat the "source" argument to mount(2) as basically a comment, and ignore it. When you're adding things like size limitations, it goes in the "data" argument (I.E. mount -o thingy). - synthetic filesystems are just interfaces to the kernel that make up their contents programmatically (proc, sys, cgroup...) and no two are alike, although they generally ignore their "source" argument and look at "data" too. I wrote up documention about this many years ago... https://landley.net/toybox/doc/mount.html > I also found that root is always mounted as rootfs in my initramfs system > which I understood to be tmpfs, is that incorrect? Yes, although the kernel tries to hide this by lying in /proc/mounts for bad reasons. > I modified the file before packing. To pack I use the following commands: > > $ ./usr/gen_initramfs.sh -l initramfs.list -e xattr ../rootfs > initramfs.cpio > $ gzip initramfs.cpio > $ mkimage -A arm64 -O linux -T ramdisk -d initramfs.cpio.gz uRamdisk > > The kernel is loaded using: > booti ${kernaddr} ${initramaddr} ${dtbaddr} Remove the root= argument from your kernel command line. It is explicitly telling the kernel "we will not be staying in rootfs" and thus it doesn't use tmpfs for it. In your case, you're saying "we're going to overmount the initial ramfs with a ram disk block device", which is nonsensical because nothing can have populated it so it will be all zeroes (unformatted) and thus the filesystem type detection staircase in https://github.com/torvalds/linux/blob/v5.18/init/do_mounts_rd.c#L38 won't be able to find a filesystem type to mount on it and it's guaranteed to fail. Note: initramfs was introduced in the early 2000s, and back in the 1990s there was an older "initrd" mechanism that DID use ramdisks (which are a chunk of ram used as a block device). I wrote documention about THAT too: https://www.kernel.org/doc/Documentation/filesystems/ramfs-rootfs-initramfs.txt Basically the mechanism you're feeding init.cpio.gz in through was originally written to populate a ramdisk, and you'd make an ext2 image or something and gzip that. These days, the kernel decompresses the first few bytes of the file and if the result is a cpio signature it calls the initramfs plumbing (extracting the archive into the ram backed filesystem) and if not it extracts it into the /dev/ram0 block device and treats it as an initial ram disk. In NEITHER case do you need root= because that's used AFTER initramfs and initrd have both failed to find an /init program. (Well initrd looks for /linuxrc instead of /init because historical cruft, and then there was pivot_root... Don't go there.) Rob
On 7/19/22 01:55, Roberto Sassu wrote: >> Thank you, I have tested that patch but the problem remained. Here is my >> command line, I wonder if there is something wrong. >> >> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 >> initrd=0x500000000 rootwait > > It is just initramtmpfs, without rootfstype=. Whoever wrote that patch really doesn't understand how this stuff works. I can tell from the name. Technically, initramfs is the loader, I.E. "init ramfs". The filesystem instance is called "rootfs" (hence the name in /proc/mounts when the insane special case the kernel added doesn't hide information from people, making all this harder to understand for no obvious reason). ramfs and tmpfs are two different filesystems that COULD be used to implement rootfs. (Last I checked they were the only ram backed filesystems in Linux.) If a system administrator says they're going to install your server's root partition using the "reiserxfs" filesystem, I would not be reassured. > Roberto Rob P.S. Note: there IS another boot option, you can have a pipe backed root filesystem! CONFIG_ROOT_NFS for NFS or CONFIG_CIFS_ROOT for Samba. No, I don't know why the order isn't consistent. P.P.S. If you want to run a command other than /init out of initramfs or initrd, use the rdinit=/run/this option. Note the root= overmount mechanism is completely different code and uses the init=/run/this argument instead, which means nothing to initramfs. Again, specifying root= says we are NOT staying in initramfs.
> From: Rob Landley [mailto:rob@landley.net] > Sent: Tuesday, July 19, 2022 1:51 PM > On 7/19/22 01:55, Roberto Sassu wrote: > >> Thank you, I have tested that patch but the problem remained. Here is my > >> command line, I wonder if there is something wrong. > >> > >> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 > >> initrd=0x500000000 rootwait > > > > It is just initramtmpfs, without rootfstype=. > > Whoever wrote that patch really doesn't understand how this stuff works. I can > tell from the name. Hi Rob surely, I should have been more careful in choosing the name of the option. > Technically, initramfs is the loader, I.E. "init ramfs". The filesystem instance > is called "rootfs" (hence the name in /proc/mounts when the insane special case > the kernel added doesn't hide information from people, making all this harder to > understand for no obvious reason). Ok, thanks for the explanation. > ramfs and tmpfs are two different filesystems that COULD be used to implement > rootfs. (Last I checked they were the only ram backed filesystems in Linux.) Yes, that part I got it. > If a system administrator says they're going to install your server's root > partition using the "reiserxfs" filesystem, I would not be reassured. Definitely. [...] > P.P.S. If you want to run a command other than /init out of initramfs or initrd, > use the rdinit=/run/this option. Note the root= overmount mechanism is > completely different code and uses the init=/run/this argument instead, which > means nothing to initramfs. Again, specifying root= says we are NOT staying in > initramfs. Sorry, it was some time ago. I have to go back and see why we needed a separate option. Maybe omitting root= was impacting on mounting the real root filesystem. Will get that information. Intuitively, given that root= is consumed for example by dracut, it seems a safer choice to have an option to explicitly choose the desired filesystem. Roberto
On 7/19/22 07:26, Roberto Sassu wrote: >> P.P.S. If you want to run a command other than /init out of initramfs or initrd, >> use the rdinit=/run/this option. Note the root= overmount mechanism is >> completely different code and uses the init=/run/this argument instead, which >> means nothing to initramfs. Again, specifying root= says we are NOT staying in >> initramfs. > > Sorry, it was some time ago. I have to go back and see why we needed > a separate option. Did I mention that init/do_mounts.c already has: __setup("rootfstype=", fs_names_setup); static char * __initdata root_fs_names; static int __init fs_names_setup(char *str) { root_fs_names = str; return 1; } void __init init_rootfs(void) { if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] && (!root_fs_names || strstr(root_fs_names, "tmpfs"))) is_tmpfs = true; } I thought I'd dealt with this back in commit 6e19eded3684? Hmmm, looks like it might need something like: diff --git a/init/do_mounts.c b/init/do_mounts.c index 7058e14ad5f7..4b4e1ffa20e1 100644 --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -665,7 +665,7 @@ struct file_system_type rootfs_fs_type = { void __init init_rootfs(void) { - if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] && - (!root_fs_names || strstr(root_fs_names, "tmpfs"))) + if (IS_ENABLED(CONFIG_TMPFS) && (!root_fs_names ? !saved_root_name[0] : + strstr(root_fs_names, "tmpfs")) is_tmpfs = true; } > Maybe omitting root= was impacting on mounting > the real root filesystem. Will get that information. I know some old bootloaders hardwire in the command line so people can't _remove_ the root=. The reason I didn't just make rootfs always be tmpfs when CONFIG_TMPFS is enabled is: A) It uses very slightly more resources, and the common case is overmounting an empty rootfs. (And then hiding it from /proc/mounts so people don't ask too many questions.) B) Some embedded systems use more than 50% of the system's memory for initramfs contents, which the tmpfs defaults won't allow (fills up at 50%), and I'm not sure I ever hooked up I don't think I ever hooked up rootflags= ala root_mount_data to the initramfs mount? (If so, setting size= through that should work...) > Intuitively, given that root= is consumed for example by dracut, it seems > a safer choice to have an option to explicitly choose the desired filesystem. Sounds like a dracut issue. Have you used dracut in a system running from initramfs? Lots of systems running from initramfs already DON'T have a root=, so you're saying dracut being broken when there is no root= is something to work around rather than fix in dracut, even though it's been easy to create a system without a root= for a decade and a half already... > Roberto Rob
> From: Rob Landley [mailto:rob@landley.net] > Sent: Tuesday, July 19, 2022 4:15 PM > On 7/19/22 07:26, Roberto Sassu wrote: > >> P.P.S. If you want to run a command other than /init out of initramfs or initrd, > >> use the rdinit=/run/this option. Note the root= overmount mechanism is > >> completely different code and uses the init=/run/this argument instead, > which > >> means nothing to initramfs. Again, specifying root= says we are NOT staying > in > >> initramfs. > > > > Sorry, it was some time ago. I have to go back and see why we needed > > a separate option. > > Did I mention that init/do_mounts.c already has: > > __setup("rootfstype=", fs_names_setup); It is consumed by dracut too, for the real root filesystem. [...] > Lots of systems running from initramfs already DON'T have a root=, so you're > saying dracut being broken when there is no root= is something to work around > rather than fix in dracut, even though it's been easy to create a system without > a root= for a decade and a half already... If there is a possibility that root= or rootfstype= are used by someone else, I would not count on those to make a selection of the filesystem for rootfs. On the other hand, what can go wrong in having a dedicated, not used by anyone option to do this job? Thanks Roberto
On 19/07/2022 07:55, Roberto Sassu wrote: >> From: Jim Baxter [mailto:jim_baxter@mentor.com] >> Sent: Monday, July 18, 2022 8:08 PM >> >> >> >> Best regards, >> >> *Jim Baxter* >> >> Siemens Digital Industries Software >> Automotive Business Unit >> DI SW STS ABU >> UK >> Tel.: +44 (161) 926-1656 >> mailto:jim.baxter@siemens.com <mailto:jim.baxter@siemens.com> >> sw.siemens.com <https://sw.siemens.com/> >> >> On 18/07/2022 17:49, Roberto Sassu wrote: >>>> From: Jim Baxter [mailto:jim_baxter@mentor.com] >>>> Sent: Monday, July 18, 2022 6:36 PM >>>> >>>> >>>> Hello, >>>> >>>> I have been testing these patches and do not see the xattr information when >>>> trying to retrieve it within the initramfs, do you have an example of how >>>> you tested this originally? >>> >>> Hi Jim, all >>> >>> apologies, I didn't find yet the time to look at this. >> >> Hello Roberto, >> >> Thank you for your response, I can wait until you have looked at the patches, >> I asked the question to make sure it was not something wrong in my >> configuration. >> >>> >>> Uhm, I guess this could be solved with: >>> >>> https://github.com/openeuler- >> mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da >>> >>> and adding initramtmpfs to the kernel command line. You are >>> probably using ramfs, which does not have xattr support. >>> Can I clarify which filesystem type is supported with this patch series? Is it tmpfs or perhaps a ramdisk? >> >> >> Thank you, I have tested that patch but the problem remained. Here is my >> command line, I wonder if there is something wrong. >> >> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 >> initrd=0x500000000 rootwait > > It is just initramtmpfs, without rootfstype=. > > Roberto Best regards, Jim
On 7/29/22 05:37, Jim Baxter wrote: >>>> Uhm, I guess this could be solved with: >>>> >>>> https://github.com/openeuler- >>> mirror/kernel/commit/18a502f7e3b1de7b9ba0c70896ce08ee13d052da >>>> >>>> and adding initramtmpfs to the kernel command line. You are >>>> probably using ramfs, which does not have xattr support. >>>> Oh, here's the actual tested version of the patch wiring up rootfstype=tmpfs to force rootfs to be tmpfs even when you specify root= diff --git a/init/do_mounts.c b/init/do_mounts.c index 7058e14ad5f7..dedf27fe9044 100644 --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -665,7 +665,7 @@ struct file_system_type rootfs_fs_type = { void __init init_rootfs(void) { - if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] && - (!root_fs_names || strstr(root_fs_names, "tmpfs"))) + if (IS_ENABLED(CONFIG_TMPFS) && (!root_fs_names ? !saved_root_name[0] : + !!strstr(root_fs_names, "tmpfs"))) is_tmpfs = true; } Signed-in-triplicate-by: Rob Landley <rob@landley.net> No idea why nobody else has fixed that bug in the past 9 years, seems obvious? Anyway, here's the testing I did using mkroot (ala https://landley.net/toybox/faq.html#mkroot): $ (cd root/x86_64; KARGS='quiet root=potato HANDOFF="/bin/head -n 1 /proc/mounts"' ./run-qemu.sh) | tail -n 3 rootfs / rootfs rw 0 0 reboot: Restarting system $ (cd root/x86_64; KARGS='quiet HANDOFF="/bin/head -n 1 /proc/mounts"' ./run-qemu.sh) | tail -n 3 rootfs / rootfs rw,size=121828k,nr_inodes=30457 0 0 reboot: Restarting system $ (cd root/x86_64; KARGS='quiet rootfstype=tmpfs root=potato HANDOFF="/bin/head -n 1 /proc/mounts"' ./run-qemu.sh) | tail -n 3 rootfs / rootfs rw,size=121828k,nr_inodes=30457 0 0 reboot: Restarting system I.E. rootfstype=tmpfs neutralized the root= so it was still tmpfs despite the kernel being explicitly told you weren't going to stay on initramfs (which is still what root= means). With just root= it's still ramfs, with all the "my log file got too big and the system livelocked" and "querying available space always returns zero" that entails. > Can I clarify which filesystem type is supported with this patch series? > Is it tmpfs or perhaps a ramdisk? I believe both tmpfs and ramfs support xattrs? (I know tmpfs does, and fs/ramfs/file-mmu.c plugs simple_getattr() into ramfs_file_operations.setattr so it looks like that would too? Haven't tried it.) This isn't a modification to the filesystem code (ramfs/tmpfs), this is a modification to the boot-time loader (initramfs) that extracts a cpio.gz file into the filesystem. Ramdisks have supported xattrs for years: they fake up a block device out of a chunk of memory and them format it and mount some other filesystem on it, meaning the driver for the other filesystem handles the xattr support. But ramdisks don't use initramfs, they load an image of the preformatted filesystem into the ramdisk block device. Completely separate mechanism, sharing no code with initramfs, depending on the block layer, etc. >>> Thank you, I have tested that patch but the problem remained. Here is my >>> command line, I wonder if there is something wrong. >>> >>> Kernel command line: rw rootfstype=initramtmpfs root=/dev/ram0 >>> initrd=0x500000000 rootwait >> >> It is just initramtmpfs, without rootfstype=. The above patch does not go on top of that patch, it's instead of. Rob