[PATCH 0/3] fs: return error pointers from ->get_link() on failure

[PATCH 0/3] fs: return error pointers from ->get_link() on failure

[Sascha Hauer]

Like in Linux, the callers of ->get_link() expect an error pointer when
the link can't be read: readlinkat() checks the result only with
IS_ERR() before copying from it, and path lookup takes NULL to mean that
the link has already been followed and continues from the directory
containing the link. barebox has no nd_jump_link(), so a NULL from
->get_link() is never legitimate there.

9p and nfs already return error pointers, and the squashfs series posted
separately converts squashfs_get_link(). This series converts the
remaining file systems:

- legacy returned NULL when the driver's readlink failed.
- cramfs and ext4 checked their read helpers for return values these
  never produce, so a link that couldn't be read was returned as a valid,
  partly zeroed link and stayed cached in inode->i_link.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (3):
      fs: cramfs: return an error pointer from cramfs_get_link()
      fs: ext4: return an error pointer from ext_get_link()
      fs: legacy: return an error pointer from legacy_get_link()

 fs/cramfs/cramfs.c    | 7 +++++--
 fs/ext4/ext_barebox.c | 3 ++-
 fs/legacy.c           | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)
---
base-commit: f2e574b9c63c6da07a06179a868cae8a471ec176
change-id: 20261102-fs-getlink-fixes-d634fd50d1f9

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

[PATCH 1/3] fs: cramfs: return an error pointer from cramfs_get_link()

[Sascha Hauer]

cramfs_get_link() checks the result of cramfs_read_file() for a negative
value, which it never returns: when a block fails to decompress, it
stops and returns how much it has copied so far. A link that can't be
read is therefore returned as if it could, with its unread part zeroed,
and stays cached in inode->i_link.

Treat a short read as -EIO, drop the buffer and return an error pointer.
Not NULL, which the check would have returned: readlinkat() checks the
link only with IS_ERR() before copying from it, and path lookup takes
NULL to mean that the link has already been followed.

Fixes: 0a5e0f40f9 ("fs: cramfs: Switch to dentry cache implementation")
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/cramfs/cramfs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/cramfs/cramfs.c b/fs/cramfs/cramfs.c
index d66cce8df8..f1048d3280 100644
--- a/fs/cramfs/cramfs.c
+++ b/fs/cramfs/cramfs.c
@@ -432,8 +432,11 @@ static const char *cramfs_get_link(struct dentry *dentry, struct inode *inode)
 	inode->i_link = xzalloc(size_add(inode->i_size, 1));
 
 	ret = cramfs_read_file(inode, 0, inode->i_link, inode->i_size);
-	if (ret < 0)
-		return NULL;
+	if (ret != inode->i_size) {
+		free(inode->i_link);
+		inode->i_link = NULL;
+		return ERR_PTR(-EIO);
+	}
 
 	return inode->i_link;
 }

-- 
2.47.3

[PATCH 2/3] fs: ext4: return an error pointer from ext_get_link()

[Sascha Hauer]

ext_get_link() checks the result of ext4fs_read_file() for 0, which it
can't return here: the symlink is at least sizeof(inode.b.symlink) bytes
long, so it returns the full size or a negative error code. On an error,
the link is therefore returned with whatever was read into it, and
stays cached in inode->i_link.

Check for a negative return value instead and return it as an error
pointer, the way readlinkat() and path lookup expect it.

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

diff --git a/fs/ext4/ext_barebox.c b/fs/ext4/ext_barebox.c
index 8f6802e94b..4035d5cf59 100644
--- a/fs/ext4/ext_barebox.c
+++ b/fs/ext4/ext_barebox.c
@@ -203,9 +203,10 @@ static const char *ext_get_link(struct dentry *dentry, struct inode *inode)
 	inode->i_link = dma_xzalloc(size_add(inode->i_size, 1));
 
 	ret = ext4fs_read_file(node, 0, inode->i_size, inode->i_link);
-	if (ret == 0) {
+	if (ret < 0) {
 		free(inode->i_link);
 		inode->i_link = NULL;
+		return ERR_PTR(ret);
 	}
 
 	return inode->i_link;

-- 
2.47.3

[PATCH 3/3] fs: legacy: return an error pointer from legacy_get_link()

[Sascha Hauer]

legacy_get_link() returns NULL when the driver's readlink fails, but
readlinkat() checks the returned link only with IS_ERR() before copying
from it, and path lookup takes NULL to mean that the link has already
been followed. Return the driver's error as an error pointer instead,
like legacy_get_link() already does when there is no readlink.

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

diff --git a/fs/legacy.c b/fs/legacy.c
index dce14706ca..07381d2b5f 100644
--- a/fs/legacy.c
+++ b/fs/legacy.c
@@ -289,7 +289,7 @@ static const char *legacy_get_link(struct dentry *dentry, struct inode *inode)
 	free(pathname);
 
 	if (ret)
-		return NULL;
+		return ERR_PTR(ret);
 
 	inode->i_link = xstrdup(link);
 

-- 
2.47.3

Re: [PATCH 0/3] fs: return error pointers from ->get_link() on failure

[Ahmad Fatoum]

Hello Sascha,

On 11/2/26 10:08 AM, Sascha Hauer wrote:
> Like in Linux, the callers of ->get_link() expect an error pointer when
> the link can't be read: readlinkat() checks the result only with
> IS_ERR() before copying from it, and path lookup takes NULL to mean that
> the link has already been followed and continues from the directory
> containing the link. barebox has no nd_jump_link(), so a NULL from
> ->get_link() is never legitimate there.
> 
> 9p and nfs already return error pointers, and the squashfs series posted
> separately converts squashfs_get_link(). This series converts the
> remaining file systems:
> 
> - legacy returned NULL when the driver's readlink failed.

What do you think, is it time we (meaning you, because you said the
l-word) do something about the legacy file systems? :D

Cheers,
Ahmad

> - cramfs and ext4 checked their read helpers for return values these
>   never produce, so a link that couldn't be read was returned as a valid,
>   partly zeroed link and stayed cached in inode->i_link.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> Sascha Hauer (3):
>       fs: cramfs: return an error pointer from cramfs_get_link()
>       fs: ext4: return an error pointer from ext_get_link()
>       fs: legacy: return an error pointer from legacy_get_link()
> 
>  fs/cramfs/cramfs.c    | 7 +++++--
>  fs/ext4/ext_barebox.c | 3 ++-
>  fs/legacy.c           | 2 +-
>  3 files changed, 8 insertions(+), 4 deletions(-)
> ---
> base-commit: f2e574b9c63c6da07a06179a868cae8a471ec176
> change-id: 20261102-fs-getlink-fixes-d634fd50d1f9
> 
> Best regards,

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/3] fs: return error pointers from ->get_link() on failure

[Sascha Hauer]

On 2026-11-02 11:21, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 11/2/26 10:08 AM, Sascha Hauer wrote:
> > Like in Linux, the callers of ->get_link() expect an error pointer when
> > the link can't be read: readlinkat() checks the result only with
> > IS_ERR() before copying from it, and path lookup takes NULL to mean that
> > the link has already been followed and continues from the directory
> > containing the link. barebox has no nd_jump_link(), so a NULL from
> > ->get_link() is never legitimate there.
> > 
> > 9p and nfs already return error pointers, and the squashfs series posted
> > separately converts squashfs_get_link(). This series converts the
> > remaining file systems:
> > 
> > - legacy returned NULL when the driver's readlink failed.
> 
> What do you think, is it time we (meaning you, because you said the
> l-word) do something about the legacy file systems? :D

Yes, sure. I can have a look into it.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 0/3] fs: return error pointers from ->get_link() on failure

[Sascha Hauer]

On Mon, 02 Nov 2026 10:08:38 +0100, Sascha Hauer wrote:
> Like in Linux, the callers of ->get_link() expect an error pointer when
> the link can't be read: readlinkat() checks the result only with
> IS_ERR() before copying from it, and path lookup takes NULL to mean that
> the link has already been followed and continues from the directory
> containing the link. barebox has no nd_jump_link(), so a NULL from
> ->get_link() is never legitimate there.
> 
> [...]

Applied, thanks!

[1/3] fs: cramfs: return an error pointer from cramfs_get_link()
      https://git.pengutronix.de/cgit/barebox/commit/?id=ec13edfe0bcb (link may not be stable)
[2/3] fs: ext4: return an error pointer from ext_get_link()
      https://git.pengutronix.de/cgit/barebox/commit/?id=abf4a2626aec (link may not be stable)
[3/3] fs: legacy: return an error pointer from legacy_get_link()
      https://git.pengutronix.de/cgit/barebox/commit/?id=6a8b1f474121 (link may not be stable)

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