| Summary: | WIC does not preserve metadata when updating fstab | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Yocto Project Subprojects] Wic | Reporter: | Daniel Andrade <daniel.andrade> | ||||||
| Component: | wic | Assignee: | Trevor Woerner <twoerner> | ||||||
| Status: | ACCEPTED --- | QA Contact: | |||||||
| Severity: | major | ||||||||
| Priority: | Medium | CC: | ccasciato, daniele.romano, hongxu.jia, randy.macleod, ross.burton, twoerner | ||||||
| Version: | v0.3.0 | ||||||||
| Target Milestone: | v0.4.0 | ||||||||
| Hardware: | All | ||||||||
| OS: | Multiple | ||||||||
| Whiteboard: | |||||||||
| OS type for building Yocto: | --- | Type of Regression: | --- | ||||||
| Verified: | Documentation change: | No (bug/feature does not impact docs) | |||||||
| Attachments: |
|
||||||||
Daniel, I've cc'ed Trevor and Hongxu who work on wic at times. Are you willing to send a patch to this list and CC these folks ? -- YP Bug review. Created attachment 5133 [details]
The patch corrects the replacement of fstab instead of the file contents
Using `install` in the rootfs plugin forces fstab to be replaced entirely, meaning that even its Inodes will change, leading xattrs and SELinux context stored by pseudo not to be applied. The fix just uses `cp` without preserving attributes from the temporary fstab since none of them are needed, just the content.
Same thing happens with the predefined mechanisms for ext4 and msdos. Using debugfs there is no way to replace contents while maintaining metadata, so the approach taken on the path was to remove the different fstab logic for those fstypes and also use the same modified cp command. Reviewing the builds I did it seems to work for all of the fstypes.
Another topic (not the real reason of this bug but also related) is that the timestamp applied to fstab is not the same as every other file. It seems like the `SOURCE_DATE_EPOCH` variable goes to the fallback timestamp (`SOURCE_DATE_EPOCH_FALLBACK`).
Since you are using that variable everywhere, it is not the same value as ` REPRODUCIBLE_TIMESTAMP_ROOTFS` under `poky/meta/conf/bitbake.conf` that is applied in every other file.
Daniel, Thanks for the attached patch. Are you willing and able to send it to the oe-core list for review ? https://docs.yoctoproject.org/contributor-guide/index.html You could include your question about the timestamp on /etc/fstab as part of your cover letter or just reply to your own thread or even make the change and see what people think in review. The bug is now being addressed at: https://lists.openembedded.org/g/openembedded-core/topic/115043355 As suggested by Randy MacLeod Patch being discuss on the list. Daniel's email bounces so move to unassigned. Add TrevorW's gmail. Trevor, do you want to comment on or own this bug? Bulk move of 6.0 Medium bugs to 6.1. Created attachment 5240 [details]
Retrive And Restore fstab xattrs patch
Hello everyone,
While reviewing this issue, I thought of a very simple and effective two-step approach with minimal code changes and that should be unlikely to have undesired effects.
1) First, we retrive the fstab xattr before replacing it with the following lines:
get_xattr_cmd = "getfattr --absolute-names -dm- %s" % (fstab_path)
ret, source_xattr = exec_native_cmd(get_xattr_cmd, native_sysroot, pseudo)
2) After the replacement of the fstab with install, we re-apply the stored xattr with the following lines:
apply_xattr_cmd = "setfattr --restore=- %s" % (f"<< 'STDIN'\n{source_xattr}\nSTDIN")
ret,out=exec_native_cmd(apply_xattr_cmd, native_sysroot, pseudo)
All the tests and test-suites performed were successful. You can find the patch attached.
Let me know if you need any additional information.
Thank you and regards,
Daniele
|
The functionality to update fstab generates a new temporary fstab with the new partition configuration. However, this file does not retain any metadata being it a completely new file. Because of this, when the rootfs plugin under `poky/scripts/lib/wic/plugins/source/rootfs.py` copies the file, it overrides the original metadata of it. 73 @classmethod 74 def do_prepare_partition(cls, part, source_params, cr, cr_workdir, 75 oe_builddir, bootimg_dir, kernel_dir, 76 krootfs_dir, native_sysroot): ... 224 if part.update_fstab_in_rootfs and part.has_fstab and not part.no_fstab_update: 225 fstab_path = os.path.join(new_rootfs, "etc/fstab") 226 # Assume that fstab should always be owned by root with fixed permissions 227 install_cmd = "install -m 0644 -p %s %s" % (part.updated_fstab_path, fstab_path) 228 if new_pseudo: 229 pseudo = cls.__get_pseudo(native_sysroot, new_rootfs, new_pseudo) 230 else: 231 pseudo = None 232 exec_native_cmd(install_cmd, native_sysroot, pseudo) A dirty fix was changing the install_cmd line to `"cat %s > %s" % (part.updated_fstab_path, fstab_path)`. However, the reason to reach this stage was that the fstype used was squashfs, because, if it is ext* or vfat, it is handled in a different location instead of here, where this fix is not doable. In `partition.py` under `poky/scripts/lib/wic/partition.py`, the original fstab is explicitly deleted, losing track of all xattrs and later it copies the temporary one into the image. By using debugfs, the "dirty fix" previously mentioned is not doable 298 if self.updated_fstab_path and self.has_fstab and not self.no_fstab_update: 299 debugfs_script_path = os.path.join(cr_workdir, "debugfs_script") 300 with open(debugfs_script_path, "w") as f: 301 f.write("cd etc\n") 302 f.write("rm fstab\n") 303 f.write("write %s fstab\n" % (self.updated_fstab_path)) 304 debugfs_cmd = "debugfs -w -f %s %s" % (debugfs_script_path, rootfs) 305 exec_native_cmd(debugfs_cmd, native_sysroot) Fixing Solution: This fstab update should be delegated to one class, in this case, Rootfs one seems the most appropriate to contain that functionality. If we still want to maintain all the code as it is, then the temporary updated fstab should inherit all the metadata of the original fstab. TL;DR: The content of the temporary updated fstab should be copied into the original fstab, preserving the metadata and xattrs, for example, SELinux xattrs. If we override the original one, we lose those.