| Message ID | 20211222083515.1150267-2-troglobit@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | package/makedevs: allow recursive on directory with symlinks | expand |
On 22/12/2021 09:35, Joachim Wiberg wrote: > When using BR2_ROOTFS_DEVICE_TABLE to change ownership of /etc, like so: > > /etc r -1 root wheel - - - - - > > makdevs fails due to it trying to chown() a symlink: The problem is actually a *dangling* symlink, not a symlink per se. > > makedevs: chown failed for /src/myLinux/output/build/buildroot-fs/ext2/target/etc/mtab: No such file or directory > makedevs: line 25: recursive failed for /src/myLinux/output/build/buildroot-fs/ext2/target/etc: No such file or directory > make[2]: *** [fs/ext2/ext2.mk:63: /src/myLinux/output/images/rootfs.ext2] Error 1 > make[1]: *** [Makefile:84: _all] Error 2 > make[1]: Leaving directory '/src/myLinux/buildroot' > > This patch changes chown() to lchown() in two cases in makedevs.c when > the argument can be a symlink. > > Signed-off-by: Joachim Wiberg <troglobit@gmail.com> > --- > package/makedevs/makedevs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c > index c57b964f5c..634ae7c3d8 100644 > --- a/package/makedevs/makedevs.c > +++ b/package/makedevs/makedevs.c > @@ -440,7 +440,7 @@ void bb_show_usage(void) > int bb_recursive(const char *fpath, const struct stat *sb, > int tflag, struct FTW *ftwbuf){ > > - if (chown(fpath, recursive_uid, recursive_gid) == -1) { > + if (lchown(fpath, recursive_uid, recursive_gid) == -1) { In this v2 you change the chown, but the chmod below still remains. Doesn't it have the same problem with a dangling symlink? Regards, Arnout > bb_perror_msg("chown failed for %s", fpath); > return -1; > } > @@ -628,7 +628,7 @@ int main(int argc, char **argv) > if (mknod(full_name_inc, mode, rdev) < 0) { > bb_perror_msg("line %d: can't create node %s", linenum, full_name_inc); > ret = EXIT_FAILURE; > - } else if (chown(full_name_inc, uid, gid) < 0) { > + } else if (lchown(full_name_inc, uid, gid) < 0) { > bb_perror_msg("line %d: can't chown %s", linenum, full_name_inc); > ret = EXIT_FAILURE; > } else if (chmod(full_name_inc, mode) < 0) { >
On 12/22/21 3:06 PM, Arnout Vandecappelle wrote: > On 22/12/2021 09:35, Joachim Wiberg wrote: >> When using BR2_ROOTFS_DEVICE_TABLE to change ownership of /etc, like so: >> /etc r -1 root wheel - - - - - >> makdevs fails due to it trying to chown() a symlink: >> makedevs: chown failed for >> /src/myLinux/output/build/buildroot-fs/ext2/target/etc/mtab: No such >> file or directory > > The problem is actually a *dangling* symlink, not a symlink per se. Ouch, you're completely right! The v1 patch was much more on point, I should know better than relying on strangers in IRC chat rooms ;-) >> - if (chown(fpath, recursive_uid, recursive_gid) == -1) { >> + if (lchown(fpath, recursive_uid, recursive_gid) == -1) { > In this v2 you change the chown, but the chmod below still remains. > Doesn't it have the same problem with a dangling symlink? Indeed it does. Seems I didn't test that case properly. I.e., worked fine in my particular use-case. I've done a separate test program to verify, and we need something similar to this at the beginning of the bb_recursive() function: if (type == FTW_SL) { if (access(fpath, F_OK)) { warn("skipping %s, dangling symlink", fpath); return 0; } } Thanks for the awesome feedback, I'll get right on a v3! Best regards /Joachim
diff --git a/package/makedevs/makedevs.c b/package/makedevs/makedevs.c index c57b964f5c..634ae7c3d8 100644 --- a/package/makedevs/makedevs.c +++ b/package/makedevs/makedevs.c @@ -440,7 +440,7 @@ void bb_show_usage(void) int bb_recursive(const char *fpath, const struct stat *sb, int tflag, struct FTW *ftwbuf){ - if (chown(fpath, recursive_uid, recursive_gid) == -1) { + if (lchown(fpath, recursive_uid, recursive_gid) == -1) { bb_perror_msg("chown failed for %s", fpath); return -1; } @@ -628,7 +628,7 @@ int main(int argc, char **argv) if (mknod(full_name_inc, mode, rdev) < 0) { bb_perror_msg("line %d: can't create node %s", linenum, full_name_inc); ret = EXIT_FAILURE; - } else if (chown(full_name_inc, uid, gid) < 0) { + } else if (lchown(full_name_inc, uid, gid) < 0) { bb_perror_msg("line %d: can't chown %s", linenum, full_name_inc); ret = EXIT_FAILURE; } else if (chmod(full_name_inc, mode) < 0) {
When using BR2_ROOTFS_DEVICE_TABLE to change ownership of /etc, like so: /etc r -1 root wheel - - - - - makdevs fails due to it trying to chown() a symlink: makedevs: chown failed for /src/myLinux/output/build/buildroot-fs/ext2/target/etc/mtab: No such file or directory makedevs: line 25: recursive failed for /src/myLinux/output/build/buildroot-fs/ext2/target/etc: No such file or directory make[2]: *** [fs/ext2/ext2.mk:63: /src/myLinux/output/images/rootfs.ext2] Error 1 make[1]: *** [Makefile:84: _all] Error 2 make[1]: Leaving directory '/src/myLinux/buildroot' This patch changes chown() to lchown() in two cases in makedevs.c when the argument can be a symlink. Signed-off-by: Joachim Wiberg <troglobit@gmail.com> --- package/makedevs/makedevs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)