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

[Stephen Warren]

On 04/27/2016 10:40 AM, Stefan Roese wrote:

From: Ronald Zachariah 

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


Reviewed-by: Stephen Warren 

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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Stefan Roese]

From: Ronald Zachariah 

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

Signed-off-by: Ronald Zachariah 
Signed-off-by: Stefan Roese 
Cc: Stephen Warren 
Cc: Tom Rini 
---
 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

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Gary,

On Tuesday, 8 September 2015, Gary Bisson
 wrote:
>
> Hi Simon,
>
> On Tue, Sep 8, 2015 at 5:56 AM, Simon Glass  wrote:
> > Hi Gary,
> >
> > On 7 September 2015 at 03:20, 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 
> >> ---
> >> 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Gary Bisson]

Hi Simon,

On Tue, Sep 8, 2015 at 5:56 AM, Simon Glass  wrote:
> Hi Gary,
>
> On 7 September 2015 at 03:20, 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 
>> ---
>> 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 mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[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 
---
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, );
-   if (status == 0) {
+   if ((status < 0) || (actread == 0)) {
free(symlink);
return 0;
}
-- 
2.5.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

[Simon Glass]

Hi Gary,

On 7 September 2015 at 03:20, 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 
> ---
> 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, );
> -   if (status == 0) {
> +   if ((status < 0) || (actread == 0)) {
> free(symlink);
> return 0;
> }
> --
> 2.5.1
>

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot