[PATCH 0/9] symlink fixes and improvements

[PATCH 0/9] symlink fixes and improvements

[Sascha Hauer]

symlinks in ramfs are created with the wrong filesize causing broken
symlinks on envfs. Looking into this revealed other shortcomings which
are fixed in this series. As a bonus the ln command gets an ignored
-s option and also the link destination can now be an existing directory
in which case it behaves like the linux ln command.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (9):
      fs: ramfs: report symlink target length as st_size
      fs: qemu_fw_cfg: report symlink target length as st_size
      fs: ramfs: create symlinks with mode 0777
      fs: follow absolute symlinks from the root directory
      fs: allow trailing symlinks pointing to the root directory
      fs: symlink: drop references taken by filename_create()
      test: self: dirfd: add tests for absolute symlinks
      commands: ln: accept -s option
      commands: ln: create link inside DEST if it is a directory

 commands/ln.c     | 51 +++++++++++++++++++++++++++++++++++++++++++---
 fs/fs.c           | 25 ++++++++++++++++++++---
 fs/qemu_fw_cfg.c  |  1 +
 fs/ramfs.c        |  4 +++-
 test/self/dirfd.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 134 insertions(+), 7 deletions(-)
---
base-commit: e65108ec0a682b53ecb9e1bebde6a37e0a99ead6
change-id: 20261020-fs-symlink-fixes-e4eb5782c22e

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 1/9] fs: ramfs: report symlink target length as st_size

[Sascha Hauer]

Since the switch to the dentry cache, ramfs symlinks report a size of 0.
Before that, st_size was the length of the link target, as on Linux and
as all other barebox filesystems that support symlinks still do.

Besides the cosmetic issue in 'ls -l', this breaks saveenv: file_action()
allocates the readlink buffer based on st_size, so every symlink in /env
is stored in the environment truncated to its first character:

  ln env/init/automount /tmp/e/foo
  saveenv /tmp/e.img /tmp/e
  loadenv /tmp/e.img /tmp/l
  ls -l /tmp/l
  l---rwx---                    0 foo -> e

Set i_size to the length of the link target again.

Fixes: b283b72639 ("fs: ramfs: Switch to dentry cache implementation")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/ramfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ramfs.c b/fs/ramfs.c
index d86bf6144e..c843df6c5c 100644
--- a/fs/ramfs.c
+++ b/fs/ramfs.c
@@ -176,6 +176,7 @@ static int ramfs_symlink(struct inode *dir, struct dentry *dentry,
 		return -ENOSPC;
 
 	inode->i_link = xstrdup(symname);
+	inode->i_size = strlen(symname);
 	d_instantiate(dentry, inode);
 
 	return 0;

-- 
2.47.3

[PATCH 2/9] fs: qemu_fw_cfg: report symlink target length as st_size

[Sascha Hauer]

The by_name/ entries are symlinks into by_key/, but their inodes are
left with i_size 0, so 'ls -l' and lstat() report a size of 0 for them.
Set i_size to the length of the link target, as other filesystems do.

Replacing the leading spaces with "../" afterwards does not change the
length of the target, so it is safe to take it right away.

Fixes: 050ea5bb93 ("fs: add qemu_fw_cfg file system")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/qemu_fw_cfg.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/qemu_fw_cfg.c b/fs/qemu_fw_cfg.c
index d0414b127c..fd16c09239 100644
--- a/fs/qemu_fw_cfg.c
+++ b/fs/qemu_fw_cfg.c
@@ -241,6 +241,7 @@ static int fw_cfg_fs_parse(struct super_block *sb)
 						    (ndirs + 1) * 3, "", buf);
 
 				parent->inode.i_link = s;
+				parent->inode.i_size = strlen(s);
 				while (*s == ' ')
 					s = mempcpy(s, "../", 3);
 			}

-- 
2.47.3

[PATCH 3/9] fs: ramfs: create symlinks with mode 0777

[Sascha Hauer]

ramfs creates symlinks with S_IRWXG only, so they show up as
'l---rwx---' in 'ls -l'. Permissions of symlinks are meaningless, Linux
always creates them with 0777. Do the same here.

Fixes: b283b72639 ("fs: ramfs: Switch to dentry cache implementation")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/ramfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ramfs.c b/fs/ramfs.c
index c843df6c5c..4c64310e22 100644
--- a/fs/ramfs.c
+++ b/fs/ramfs.c
@@ -171,7 +171,7 @@ static int ramfs_symlink(struct inode *dir, struct dentry *dentry,
 {
 	struct inode *inode;
 
-	inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK | S_IRWXG);
+	inode = ramfs_get_inode(dir->i_sb, dir, S_IFLNK | S_IRWXUGO);
 	if (!inode)
 		return -ENOSPC;
 

-- 
2.47.3

[PATCH 4/9] fs: follow absolute symlinks from the root directory

[Sascha Hauer]

get_link() skips the leading slashes of an absolute symlink target, but
never moves the lookup to the root directory. The target is therefore
resolved relative to the directory containing the link:

  ln -s /tmp/a /tmp/b
  cat /tmp/b        -> looks up /tmp/tmp/a, fails with -ENOENT
  ln -s /a /tmp/z
  cat /tmp/z        -> opens /tmp/a

Do what Linux does in nd_jump_root() and replace the current path with
the root before continuing the walk. For lookups relative to an
O_CHROOT dirfd the root is the root of the dirfd's mount, so record the
root vfsmount along with the root dentry in struct nameidata.

Fixes: b3fbfad7ae ("fs: dentry cache implementation")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index 1426ce4bc8..edb68cf455 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -1726,6 +1726,7 @@ struct nameidata {
 	struct filename	*name;
 	struct inode	*link_inode;
 	struct dentry   *d_root;
+	struct vfsmount *mnt_root;
 };
 
 struct filename {
@@ -1740,6 +1741,7 @@ static void set_nameidata(struct nameidata *p, struct filename *name)
 	p->name = name;
 	p->total_link_count = 0;
 	p->d_root = d_root;
+	p->mnt_root = mnt_root;
 }
 
 static void path_get(const struct path *path)
@@ -2003,6 +2005,19 @@ static inline void path_to_nameidata(const struct path *path,
 	nd->path.dentry = path->dentry;
 }
 
+static void nd_jump_root(struct nameidata *nd)
+{
+	struct path root = {
+		.mnt = nd->mnt_root,
+		.dentry = nd->d_root,
+	};
+
+	path_get(&root);
+	path_put(&nd->path);
+	nd->path = root;
+	nd->flags |= LOOKUP_JUMPED;
+}
+
 static const char *get_link(struct nameidata *nd)
 {
 	struct saved *last = nd->stack + nd->depth - 1;
@@ -2018,6 +2033,7 @@ static const char *get_link(struct nameidata *nd)
 			return res;
 	}
 	if (*res == '/') {
+		nd_jump_root(nd);
 		while (unlikely(*++res == '/'))
 			;
 	}
@@ -2323,8 +2339,10 @@ static const char *path_init(int dirfd, struct nameidata *nd, unsigned flags)
 
 		if (*s == '/')
 			nd->path.dentry = nd->path.mnt->mnt_root;
-		if (chroot)
+		if (chroot) {
 			nd->d_root = nd->path.mnt->mnt_root;
+			nd->mnt_root = nd->path.mnt;
+		}
 	}
 
 	return s;

-- 
2.47.3

[PATCH 5/9] fs: allow trailing symlinks pointing to the root directory

[Sascha Hauer]

When the target of a trailing symlink consists only of slashes,
get_link() moves the lookup to the root directory and returns NULL, and
trailing_symlink() hands an empty remaining path to link_path_walk().
link_path_walk() rejects it with -ENOENT, so any lookup of such a link
fails:

  ln -s / /tmp/root
  ls /tmp/root      -> No such file or directory

The -ENOENT was added in commit 74946415a5 ("fs: Fix link_path_walk to
return -ENOENT on empty path") to fix a crash with 'cat ""'. Shortly
after, commit ef98a567d7 ("fs: check for empty name in getname()")
started rejecting empty names in getname(), which all lookups go through,
so the empty path from a symlink is the only one that still reaches
link_path_walk(). Drop the check again, as Linux does. 'cat ""' still
fails with -ENOENT, from getname().

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/fs.c b/fs/fs.c
index edb68cf455..40dc32b3c4 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2195,8 +2195,6 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 	int err;
 	char separator = '/';
 
-	if (!*name)
-		return -ENOENT;
 	while (*name=='/')
 		name++;
 	if (!*name)

-- 
2.47.3

[PATCH 6/9] fs: symlink: drop references taken by filename_create()

[Sascha Hauer]

symlink() never releases the dentry and the parent path returned by
filename_create(). The leaked vfsmount reference keeps the filesystem
busy, so it can't be unmounted once a symlink has been created on it:

  mount -t ramfs none /m
  ln -s x /m/l
  umount /m         -> Device or resource busy

Release both references like mkdirat() and mknodat() do. Also pin the
new dentry in ramfs_symlink() like ramfs_mknod() does, so that symlink
dentries end up with the same reference count as all other ramfs
dentries.

Fixes: b3fbfad7ae ("fs: dentry cache implementation")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c    | 3 +++
 fs/ramfs.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 40dc32b3c4..dd064c8527 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2973,6 +2973,9 @@ int symlink(const char *pathname, const char *newpath)
 	}
 
 	error = vfs_symlink(path.dentry->d_inode, dentry, pathname);
+
+	dput(dentry);
+	path_put(&path);
 out:
 	return errno_set(error);
 }
diff --git a/fs/ramfs.c b/fs/ramfs.c
index 4c64310e22..22fe620b55 100644
--- a/fs/ramfs.c
+++ b/fs/ramfs.c
@@ -178,6 +178,7 @@ static int ramfs_symlink(struct inode *dir, struct dentry *dentry,
 	inode->i_link = xstrdup(symname);
 	inode->i_size = strlen(symname);
 	d_instantiate(dentry, inode);
+	dget(dentry);   /* Extra count - pin the dentry in core */
 
 	return 0;
 }

-- 
2.47.3

[PATCH 7/9] test: self: dirfd: add tests for absolute symlinks

[Sascha Hauer]

Add tests for resolving absolute symlinks, both relative to the regular
root directory and relative to an O_CHROOT dirfd, where they must be
resolved from the root of the mount. This includes links pointing to the
root directory itself, both as last and as intermediate path component.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 test/self/dirfd.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/test/self/dirfd.c b/test/self/dirfd.c
index 04cbf6e285..be927f0761 100644
--- a/test/self/dirfd.c
+++ b/test/self/dirfd.c
@@ -96,6 +96,64 @@ static void do_test_dirfd(const char *at, int dirfd,
 }
 
 
+static void check_link(const char *at, int dirfd, const char *path,
+		       const struct stat *target)
+{
+	struct stat s;
+	int ret;
+
+	ret = statat(dirfd, path, &s);
+	if (!expect(ret == 0, target != NULL, "statat(%s, %s): %m", at, path))
+		return;
+
+	if (target)
+		expect(s.st_ino == target->st_ino, true,
+		       "statat(%s, %s) resolved to the wrong file", at, path);
+}
+
+static void test_dirfd_symlinks(void)
+{
+	struct stat root, mntroot, testfile;
+	int fd, ret;
+
+	ret = mkdir("/dirfdtest/sub", 0777);
+	if (!expect(ret == 0, true, "mkdir(\"/dirfdtest/sub\") = %d", ret))
+		return;
+
+	ret = symlink("/testfile", "/dirfdtest/sub/abslink");
+	expect(ret == 0, true, "symlink(/testfile) = %d", ret);
+	ret = symlink("/dirfdtest/testfile", "/dirfdtest/sub/fulllink");
+	expect(ret == 0, true, "symlink(/dirfdtest/testfile) = %d", ret);
+	ret = symlink("/", "/dirfdtest/sub/rootlink");
+	expect(ret == 0, true, "symlink(/) = %d", ret);
+
+	if (!expect(stat("/", &root), 0, "stat(/)") ||
+	    !expect(stat("/dirfdtest", &mntroot), 0, "stat(/dirfdtest)") ||
+	    !expect(stat("/dirfdtest/testfile", &testfile), 0,
+		    "stat(/dirfdtest/testfile)"))
+		return;
+
+	/* absolute symlinks are resolved from the root directory... */
+	check_link("AT_FDCWD", AT_FDCWD, "/dirfdtest/sub/abslink", NULL);
+	check_link("AT_FDCWD", AT_FDCWD, "/dirfdtest/sub/fulllink", &testfile);
+	check_link("AT_FDCWD", AT_FDCWD, "/dirfdtest/sub/rootlink", &root);
+	check_link("AT_FDCWD", AT_FDCWD, "/dirfdtest/sub/rootlink/", &root);
+	check_link("AT_FDCWD", AT_FDCWD,
+		   "/dirfdtest/sub/rootlink/dirfdtest/testfile", &testfile);
+
+	/* ...which is the root of the mount for O_CHROOT */
+	fd = open("/dirfdtest", O_PATH | O_CHROOT);
+	if (!expect(fd >= 0, true, "open(/dirfdtest, O_PATH | O_CHROOT) = %d", fd))
+		return;
+
+	check_link("/dirfdtest O_CHROOT", fd, "sub/abslink", &testfile);
+	check_link("/dirfdtest O_CHROOT", fd, "sub/fulllink", NULL);
+	check_link("/dirfdtest O_CHROOT", fd, "sub/rootlink", &mntroot);
+	check_link("/dirfdtest O_CHROOT", fd, "sub/rootlink/testfile", &testfile);
+
+	close(fd);
+}
+
 static void test_dirfd(void)
 {
 	int fd, ret;
@@ -116,6 +174,8 @@ static void test_dirfd(void)
 	if (!expect(ret == 0, true, "write_file() = %d", ret))
 		goto out;
 
+	test_dirfd_symlinks();
+
 #define B(dot, dotdot, zero, dev) 0b##dev##zero##dotdot##dot
 	/* We do fiften tests for every configuration
 	 * for dir in ./ /dirfdtest / ../ ; do

-- 
2.47.3

[PATCH 8/9] commands: ln: accept -s option

[Sascha Hauer]

All links barebox creates are symbolic links, but ln does not accept
the -s option, so the common 'ln -s SRC DEST' fails with a usage error.
Accept -s and ignore it, so that users and scripts can use the familiar
syntax.

While at it, add a help text for the command.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/ln.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/commands/ln.c b/commands/ln.c
index 9206480300..c907b306c9 100644
--- a/commands/ln.c
+++ b/commands/ln.c
@@ -9,22 +9,46 @@
 #include <environment.h>
 #include <fs.h>
 #include <errno.h>
+#include <getopt.h>
 
 static int do_ln(int argc, char *argv[])
 {
-	if (argc != 3)
+	int opt;
+
+	while ((opt = getopt(argc, argv, "s")) > 0) {
+		switch (opt) {
+		case 's':
+			/* all links are symbolic, accept -s for compatibility */
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
+		}
+	}
+
+	argc -= optind;
+	argv += optind;
+
+	if (argc != 2)
 		return COMMAND_ERROR_USAGE;
 
-	if (symlink(argv[1], argv[2]) < 0) {
+	if (symlink(argv[0], argv[1]) < 0) {
 		perror("ln");
 		return 1;
 	}
 	return 0;
 }
 
+BAREBOX_CMD_HELP_START(ln)
+BAREBOX_CMD_HELP_TEXT("Create a symbolic link DEST pointing to SRC.")
+BAREBOX_CMD_HELP_TEXT("")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT("-s", "ignored, all links are symbolic links")
+BAREBOX_CMD_HELP_END
+
 BAREBOX_CMD_START(ln)
 	.cmd		= do_ln,
 	BAREBOX_CMD_DESC("create symlink (make a new name for a file)")
-	BAREBOX_CMD_OPTS("SRC DEST")
+	BAREBOX_CMD_OPTS("[-s] SRC DEST")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
+	BAREBOX_CMD_HELP(cmd_ln_help)
 BAREBOX_CMD_END

-- 
2.47.3

[PATCH 9/9] commands: ln: create link inside DEST if it is a directory

[Sascha Hauer]

With Linux ln, when the link name is an existing directory, the link is
created inside that directory and named after the last component of the
target, so that 'ln -s /etc/foo .' creates ./foo. barebox ln instead
tries to create the link at DEST itself and fails with -EEXIST.

Do the same as Linux. Trailing slashes in the target are ignored for the
link name, but are kept in the target the link points to.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/ln.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/commands/ln.c b/commands/ln.c
index c907b306c9..1b18dfcf63 100644
--- a/commands/ln.c
+++ b/commands/ln.c
@@ -10,10 +10,14 @@
 #include <fs.h>
 #include <errno.h>
 #include <getopt.h>
+#include <libbb.h>
 
 static int do_ln(int argc, char *argv[])
 {
-	int opt;
+	const char *target, *dest;
+	char *linkname = NULL;
+	struct stat s;
+	int opt, ret;
 
 	while ((opt = getopt(argc, argv, "s")) > 0) {
 		switch (opt) {
@@ -31,7 +35,22 @@ static int do_ln(int argc, char *argv[])
 	if (argc != 2)
 		return COMMAND_ERROR_USAGE;
 
-	if (symlink(argv[0], argv[1]) < 0) {
+	target = argv[0];
+	dest = argv[1];
+
+	/* like Linux, create the link inside DEST when it is a directory */
+	if (!stat(dest, &s) && S_ISDIR(s.st_mode)) {
+		char *tmp = xstrdup(target);
+
+		linkname = concat_path_file(dest, posix_basename(tmp));
+		free(tmp);
+		dest = linkname;
+	}
+
+	ret = symlink(target, dest);
+	free(linkname);
+
+	if (ret < 0) {
 		perror("ln");
 		return 1;
 	}
@@ -39,7 +58,9 @@ static int do_ln(int argc, char *argv[])
 }
 
 BAREBOX_CMD_HELP_START(ln)
-BAREBOX_CMD_HELP_TEXT("Create a symbolic link DEST pointing to SRC.")
+BAREBOX_CMD_HELP_TEXT("Create a symbolic link DEST pointing to SRC. If DEST is an existing")
+BAREBOX_CMD_HELP_TEXT("directory, the link is created in it, named after the last component")
+BAREBOX_CMD_HELP_TEXT("of SRC.")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT("-s", "ignored, all links are symbolic links")

-- 
2.47.3