[U-Boot] [PATCH] fs: ext4: fix symlink read function

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Gary Bisson]

Since last API changes for files >2GB, the read of symlink is broken as
ext4fs_read_file now returns 0 instead of the length of the actual read.

Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
---
Hi all,

Switching from an old v2014.07 to v2015.07 we've noticed that we couldn't
read symlinks any more. This is due to the API changes made to
ext4fs_read_file (commit 9f12cd0).

This patch makes the read symlink check both for errors (status < 0) and
actual read length.

Regards,
Gary
---
 fs/ext4/ext4_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index cab5465..e2ab145 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2040,7 +2040,7 @@ static char *ext4fs_read_symlink(struct ext2fs_node *node)
 		status = ext4fs_read_file(diro, 0,
 					   __le32_to_cpu(diro->inode.size),
 					   symlink, &actread);
-		if (status == 0) {
+		if ((status < 0) || (actread == 0)) {
 			free(symlink);
 			return 0;
 		}
-- 
2.5.1

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Simon Glass]

Hi Gary,

On 7 September 2015 at 03:20, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:
> Since last API changes for files >2GB, the read of symlink is broken as
> ext4fs_read_file now returns 0 instead of the length of the actual read.
>
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> ---
> Hi all,
>
> Switching from an old v2014.07 to v2015.07 we've noticed that we couldn't
> read symlinks any more. This is due to the API changes made to
> ext4fs_read_file (commit 9f12cd0).
>
> This patch makes the read symlink check both for errors (status < 0) and
> actual read length.
>
> Regards,
> Gary

Thanks for fixing this. Does the filesystem test (test/fs-test.sh)
show this error? How could we enhance the test to detect this?

> ---
>  fs/ext4/ext4_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index cab5465..e2ab145 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -2040,7 +2040,7 @@ static char *ext4fs_read_symlink(struct ext2fs_node *node)
>                 status = ext4fs_read_file(diro, 0,
>                                            __le32_to_cpu(diro->inode.size),
>                                            symlink, &actread);
> -               if (status == 0) {
> +               if ((status < 0) || (actread == 0)) {
>                         free(symlink);
>                         return 0;
>                 }
> --
> 2.5.1
>

Regards,
Simon

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Gary Bisson]

Hi Simon,

On Tue, Sep 8, 2015 at 5:56 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Gary,
>
> On 7 September 2015 at 03:20, Gary Bisson
> <gary.bisson@boundarydevices.com> wrote:
>> Since last API changes for files >2GB, the read of symlink is broken as
>> ext4fs_read_file now returns 0 instead of the length of the actual read.
>>
>> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
>> ---
>> Hi all,
>>
>> Switching from an old v2014.07 to v2015.07 we've noticed that we couldn't
>> read symlinks any more. This is due to the API changes made to
>> ext4fs_read_file (commit 9f12cd0).
>>
>> This patch makes the read symlink check both for errors (status < 0) and
>> actual read length.
>>
>> Regards,
>> Gary
>
> Thanks for fixing this. Does the filesystem test (test/fs-test.sh)
> show this error? How could we enhance the test to detect this?

I wasn't aware of this testing script, a quick look at it makes me
think it doesn't check anything symlink-related. I guess it would
require to add SYMLINK_FILE that points to either SMALL_FILE or
BIG_FILE, read it and check the md5sum.

Regards,
Gary

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Simon Glass]

Hi Gary,

On Tuesday, 8 September 2015, Gary Bisson
<gary.bisson@boundarydevices.com> wrote:
>
> Hi Simon,
>
> On Tue, Sep 8, 2015 at 5:56 AM, Simon Glass <sjg@chromium.org> wrote:
> > Hi Gary,
> >
> > On 7 September 2015 at 03:20, Gary Bisson
> > <gary.bisson@boundarydevices.com> wrote:
> >> Since last API changes for files >2GB, the read of symlink is broken as
> >> ext4fs_read_file now returns 0 instead of the length of the actual read.
> >>
> >> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>
> >> ---
> >> Hi all,
> >>
> >> Switching from an old v2014.07 to v2015.07 we've noticed that we couldn't
> >> read symlinks any more. This is due to the API changes made to
> >> ext4fs_read_file (commit 9f12cd0).
> >>
> >> This patch makes the read symlink check both for errors (status < 0) and
> >> actual read length.
> >>
> >> Regards,
> >> Gary
> >
> > Thanks for fixing this. Does the filesystem test (test/fs-test.sh)
> > show this error? How could we enhance the test to detect this?
>
> I wasn't aware of this testing script, a quick look at it makes me
> think it doesn't check anything symlink-related. I guess it would
> require to add SYMLINK_FILE that points to either SMALL_FILE or
> BIG_FILE, read it and check the md5sum.


Yes I think that will work. Are you going to take a look?

Regards,
Simon
>
>

[U-Boot] fs: ext4: fix symlink read function

[Tom Rini]

On Mon, Sep 07, 2015 at 11:20:07AM +0200, Gary Bisson wrote:

> Since last API changes for files >2GB, the read of symlink is broken as
> ext4fs_read_file now returns 0 instead of the length of the actual read.
> 
> Signed-off-by: Gary Bisson <gary.bisson@boundarydevices.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150912/feaa83a1/attachment.sig>

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Stephen Warren]

On 04/27/2016 10:40 AM, Stefan Roese wrote:
> From: Ronald Zachariah <rozachar@cisco.com>
>
> The function ext4fs_read_symlink was unable to handle a symlink
> which had target name of exactly 60 characters.

Reviewed-by: Stephen Warren <swarren@nvidia.com>

This seems to match how the Linux kernel encodes symlinks.

> -	if (__le32_to_cpu(diro->inode.size) <= 60) {
> +	if (__le32_to_cpu(diro->inode.size) < 60) {

It'd be nice if "60" was replaced with sizeof(diro->inode.b.symlink), 
but that's probably a topic for a different patch.

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Stefan Roese]

From: Ronald Zachariah <rozachar@cisco.com>

The function ext4fs_read_symlink was unable to handle a symlink
which had target name of exactly 60 characters.

Signed-off-by: Ronald Zachariah <rozachar@cisco.com>
Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Tom Rini <trini@konsulko.com>
---
 fs/ext4/ext4_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index 84fba76..868c281 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2040,7 +2040,7 @@ static char *ext4fs_read_symlink(struct ext2fs_node *node)
 	if (!symlink)
 		return 0;
 
-	if (__le32_to_cpu(diro->inode.size) <= 60) {
+	if (__le32_to_cpu(diro->inode.size) < 60) {
 		strncpy(symlink, diro->inode.b.symlink,
 			 __le32_to_cpu(diro->inode.size));
 	} else {
-- 
2.8.1