Re: [PATCH] Fix possible memory corruption in xfs_readlink

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Christoph Hellwig]

This generally good, but you'll need to fix formatting a bit
for both the mail body and the patch itself.

On Mon, Oct 17, 2011 at 01:30:12PM -0200, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link
> is larger than MAXPATHLEN and XFS_DEBUG is not
> enabled.
> This also uses S_IFLNK to check link not only
> in DEBUG mode.

Please try to fill up ~ 75 characters for each line in the mail body,
e.g.

Fix a possible memory corruption when a symlink target is larger than
MAXPATHLEN and XFS_DEBUG is not enabled.  Also use S_IFLNK to check
against disk corruption in di_mode for non-debug mode.

(I've also update the content a little bit).

> -	ASSERT(S_ISLNK(ip->i_d.di_mode));
> -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
> +	if (!(S_ISLNK(ip->i_d.di_mode)) || !(ip->i_d.di_size <= MAXPATHLEN )){
> +
> +		xfs_emerg(mp, "inode (%lld), link too long or not a link",
> +			 (unsigned long long)ip->i_ino);
> +		ASSERT(0);
> +		return XFS_ERROR(EFSCORRUPTED);
> +	}

No need for the inner braces in both branches, but per kernel coding
style there should be one before the opening brace.  Also no spaces
before the closing round braces, please.  I also think it would be
cleanrer to split this into two checks, as it's two possible
corruptions, e.g.

	if (!S_ISLNK(ip->i_d.di_mode)) {
		xfs_emerg(mp, "inode (%lld) not a link in %s\n",
			  (unsigned long long)ip->i_ino), __func__);
		ASSERT(0);
		return XFS_ERROR(EFSCORRUPTED);
	}
	if (ip->i_d.di_size > MAXPATHLEN) {
		xfs_emerg(mp, "inode (%lld) larger than MAXPATHLEN in %s\n",
			  (unsigned long long)ip->i_ino), __func__);
		ASSERT(0);
		return XFS_ERROR(EFSCORRUPTED);
	}

It might also be useful to print the length in the second case as that
would help debugging potential corruptions. (e.g. single bit flips)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] Fix possible memory corruption in xfs_readlink

[Carlos Maiolino]

Fixes a possible memory corruption when the link
is larger than MAXPATHLEN and XFS_DEBUG is not
enabled.
This also uses S_IFLNK to check link not only
in DEBUG mode.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_vnodeops.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 51fc429..3bc4fda 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -123,8 +123,13 @@ xfs_readlink(
 
 	xfs_ilock(ip, XFS_ILOCK_SHARED);
 
-	ASSERT(S_ISLNK(ip->i_d.di_mode));
-	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
+	if (!(S_ISLNK(ip->i_d.di_mode)) || !(ip->i_d.di_size <= MAXPATHLEN )){
+
+		xfs_emerg(mp, "inode (%lld), link too long or not a link",
+			 (unsigned long long)ip->i_ino);
+		ASSERT(0);
+		return XFS_ERROR(EFSCORRUPTED);
+	}
 
 	pathlen = ip->i_d.di_size;
 	if (!pathlen)
-- 
1.7.6.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Eric Sandeen]

On 10/17/11 9:00 AM, Christoph Hellwig wrote:
> This generally good, but you'll need to fix formatting a bit
> for both the mail body and the patch itself.
> 
> On Mon, Oct 17, 2011 at 01:30:12PM -0200, Carlos Maiolino wrote:
>> Fixes a possible memory corruption when the link
>> is larger than MAXPATHLEN and XFS_DEBUG is not
>> enabled.
>> This also uses S_IFLNK to check link not only
>> in DEBUG mode.
> 
> Please try to fill up ~ 75 characters for each line in the mail body,
> e.g.
> 
> Fix a possible memory corruption when a symlink target is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled.  Also use S_IFLNK to check
> against disk corruption in di_mode for non-debug mode.
> 
> (I've also update the content a little bit).
> 
>> -	ASSERT(S_ISLNK(ip->i_d.di_mode));
>> -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
>> +	if (!(S_ISLNK(ip->i_d.di_mode)) || !(ip->i_d.di_size <= MAXPATHLEN )){
>> +
>> +		xfs_emerg(mp, "inode (%lld), link too long or not a link",
>> +			 (unsigned long long)ip->i_ino);
>> +		ASSERT(0);
>> +		return XFS_ERROR(EFSCORRUPTED);
>> +	}
> 
> No need for the inner braces in both branches, but per kernel coding
> style there should be one before the opening brace.  Also no spaces
> before the closing round braces, please.  I also think it would be
> cleanrer to split this into two checks, as it's two possible
> corruptions, e.g.
> 
> 	if (!S_ISLNK(ip->i_d.di_mode)) {
> 		xfs_emerg(mp, "inode (%lld) not a link in %s\n",
> 			  (unsigned long long)ip->i_ino), __func__);
> 		ASSERT(0);
> 		return XFS_ERROR(EFSCORRUPTED);
> 	}


We could get here via xfs_readlink_by_handle, but that tests
S_ISLNK(dentry->d_inode->i_mode) before calling xfs_readlink.

I'm guessing that we wouldn't get here through normal paths
if the inode in question weren't a symlink, so is there any need
to re-test ip->i_d.di_mode here at runtime?

I'm not sure both ASSERTS necessarily need to be turned into runtime tests.
The 2nd does, clearly, so we don't overflow the buffer. Just not sure about
the first.

> 	if (ip->i_d.di_size > MAXPATHLEN) {
> 		xfs_emerg(mp, "inode (%lld) larger than MAXPATHLEN in %s\n",
> 			  (unsigned long long)ip->i_ino), __func__);
> 		ASSERT(0);
> 		return XFS_ERROR(EFSCORRUPTED);
> 	}

Since we assign "pathlen" a little later, it might be prettier to use that
variable at that point?  It's there, and it's descriptive.

Also, the current xfs_emerg() convention seems to be:

function: problem description

It might we worth following that, i.e.

xfs_emerg(mp, "%s: symlink target in inode %lld too long (%lld)", __func__, ip->i_ino, pathlen)
 
-Eric

> It might also be useful to print the length in the second case as that
> would help debugging potential corruptions. (e.g. single bit flips)
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Alex Elder]

On Wed, 2011-11-02 at 15:45 -0400, Christoph Hellwig wrote:
> We should validate that the value isn't negative in xfs_iformat_*,
> although we currently don't do that.  It already verified that it
> fits into the XFS_DFORK_DSIZE, which should take care of fitting
> into 32-bits.  Adding another explicit check probably won't hurt,
> given that XFS_DFORK_DSIZE is calculated dynamically based on the
> fork offset.
> 

That's true, but there are other places where it gets
updated, yet not defensively validated.  For example,
in xfs_dir2_shrink_inode(), if:
    fsbno > (INT64_MAX >> mp->m_sb.sb_blocklog)
then the (signed) di_size field would be assigned
a value that exceeded its max representable value,
producing unreliable (implementation-defined) results.
That may well be an impossible situation, but it's
not obvious without really looking at the code.

It's a bit of a can of worms, which is why I suggested
just testing for this (unlikely) condition in
xfs_readlink() for now.

					-Alex

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Christoph Hellwig]

We should validate that the value isn't negative in xfs_iformat_*,
although we currently don't do that.  It already verified that it
fits into the XFS_DFORK_DSIZE, which should take care of fitting
into 32-bits.  Adding another explicit check probably won't hurt,
given that XFS_DFORK_DSIZE is calculated dynamically based on the
fork offset.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Alex Elder]

I don't konw why, but I *think* the response I
thought I sent earlier didn't actually make it
out.

Just in case, I'm trying to recreate what I had
before, below.  Sorry if something like this
shows up twice.

On Tue, 2011-11-01 at 14:14 +0000, Ben Hutchings wrote:
> On Tue, 2011-10-18 at 02:18 -0200, Carlos Maiolino wrote:
> > Fixes a possible memory corruption when the link is larger than
> > MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> > S_ISLNK assert, since the inode mode is checked previously in
> > xfs_readlink_by_handle() and via VFS.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> >  fs/xfs/xfs_vnodeops.c |   11 ++++++++---
> >  1 files changed, 8 insertions(+), 3 deletions(-)

A few comments inline below, followed by Ben's original
message and some explanation from me.

> > diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> > index 51fc429..c3288be 100644
> > --- a/fs/xfs/xfs_vnodeops.c
> > +++ b/fs/xfs/xfs_vnodeops.c
> > @@ -123,13 +123,18 @@ xfs_readlink(
> >  
> >  	xfs_ilock(ip, XFS_ILOCK_SHARED);
> >  
> > -	ASSERT(S_ISLNK(ip->i_d.di_mode));
> > -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
> > -
> >  	pathlen = ip->i_d.di_size;
> pathlen is a signed int (32-bit) and di_size has signed 64-bit type.

I concur, di_size here is an xfs_fsize_t, which is defined
as __int64_t (a signed 64-bit integer).  pathlen is defined
as a (signed) int.

> So, even if di_size was verified to be non-negative earlier (is it?)...

More on this question below.

> >  	if (!pathlen)
> >  		goto out;
> >  
> > +	if (pathlen > MAXPATHLEN) {
> 
> ...pathlen may be negative here and will pass this check.
> 
> Ben.

You are right to call attention to this.  I think defining
pathlen to be an int here is a mistake in any case (the type
ought to match that of id.di_size), though in practice it
will not be a problem.

You mention two remaining issues:
- can a value held in ip->i_d.di_size result in a negative
  value in pathlen as a result of the assignment?
- is ip->i_d.di_size guaranteed (verified) to be non-negative?

On the first question, the C standard says that the result of
the assignment--if id.di_size exceeds what can be represented
by pathlen--is implementation defined, therefore it is not
safe.  So you're right, this needs to be fixed.

On the second question, ip->i_d.di_size is assigned
in a lot of places.  I started looking at all the
places where this field gets assigned.  In about half
of them I examined the assignment obviously left its
value non-negative, or only allowing a negative assignment
if the previous value was already negative.

But rather than complete this research task, I think
it will be better (for now) to simply check for a negative
ip->i_d.di_size, and if it's seen, either return
an error or initiate a forced shutdown (since it
represents corruption).

I'm interested in what others think.

					-Alex

> > +		xfs_alert(mp, "%s: inode (%llu) symlink length (%d) too long",
> > +			 __func__, (unsigned long long)ip->i_ino, pathlen);
> > +		ASSERT(0);
> > +		return XFS_ERROR(EFSCORRUPTED);
> > +	}
> > +
> > +
> >  	if (ip->i_df.if_flags & XFS_IFINLINE) {
> >  		memcpy(link, ip->i_df.if_u1.if_data, pathlen);
> >  		link[pathlen] = '\0';
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs





_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Ben Hutchings]

[-- Attachment #1.1: Type: text/plain, Size: 1528 bytes --]

On Tue, 2011-10-18 at 02:18 -0200, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> S_ISLNK assert, since the inode mode is checked previously in
> xfs_readlink_by_handle() and via VFS.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/xfs_vnodeops.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 51fc429..c3288be 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -123,13 +123,18 @@ xfs_readlink(
>  
>  	xfs_ilock(ip, XFS_ILOCK_SHARED);
>  
> -	ASSERT(S_ISLNK(ip->i_d.di_mode));
> -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
> -
>  	pathlen = ip->i_d.di_size;

pathlen is a signed int (32-bit) and di_size has signed 64-bit type.
So, even if di_size was verified to be non-negative earlier (is it?)...

>  	if (!pathlen)
>  		goto out;
>  
> +	if (pathlen > MAXPATHLEN) {

...pathlen may be negative here and will pass this check.

Ben.

> +		xfs_alert(mp, "%s: inode (%llu) symlink length (%d) too long",
> +			 __func__, (unsigned long long)ip->i_ino, pathlen);
> +		ASSERT(0);
> +		return XFS_ERROR(EFSCORRUPTED);
> +	}
> +
> +
>  	if (ip->i_df.if_flags & XFS_IFINLINE) {
>  		memcpy(link, ip->i_df.if_u1.if_data, pathlen);
>  		link[pathlen] = '\0';

-- 
Ben Hutchings
Computers are not intelligent.	They only think they are.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Eric Sandeen]

On 10/17/11 11:18 PM, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> S_ISLNK assert, since the inode mode is checked previously in
> xfs_readlink_by_handle() and via VFS.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Thanks!

Discussed-to-death-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  fs/xfs/xfs_vnodeops.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 51fc429..c3288be 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -123,13 +123,18 @@ xfs_readlink(
>  
>  	xfs_ilock(ip, XFS_ILOCK_SHARED);
>  
> -	ASSERT(S_ISLNK(ip->i_d.di_mode));
> -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
> -
>  	pathlen = ip->i_d.di_size;
>  	if (!pathlen)
>  		goto out;
>  
> +	if (pathlen > MAXPATHLEN) {
> +		xfs_alert(mp, "%s: inode (%llu) symlink length (%d) too long",
> +			 __func__, (unsigned long long)ip->i_ino, pathlen);
> +		ASSERT(0);
> +		return XFS_ERROR(EFSCORRUPTED);
> +	}
> +
> +
>  	if (ip->i_df.if_flags & XFS_IFINLINE) {
>  		memcpy(link, ip->i_df.if_u1.if_data, pathlen);
>  		link[pathlen] = '\0';

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Alex Elder]

On Tue, 2011-10-18 at 02:18 -0200, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> S_ISLNK assert, since the inode mode is checked previously in
> xfs_readlink_by_handle() and via VFS.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

OK, looks good.  I'll commit it soon.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] Fix possible memory corruption in xfs_readlink

[Carlos Maiolino]

Fixes a possible memory corruption when the link is larger than
MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
S_ISLNK assert, since the inode mode is checked previously in
xfs_readlink_by_handle() and via VFS.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_vnodeops.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 51fc429..c3288be 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -123,13 +123,18 @@ xfs_readlink(
 
 	xfs_ilock(ip, XFS_ILOCK_SHARED);
 
-	ASSERT(S_ISLNK(ip->i_d.di_mode));
-	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
-
 	pathlen = ip->i_d.di_size;
 	if (!pathlen)
 		goto out;
 
+	if (pathlen > MAXPATHLEN) {
+		xfs_alert(mp, "%s: inode (%llu) symlink length (%d) too long",
+			 __func__, (unsigned long long)ip->i_ino, pathlen);
+		ASSERT(0);
+		return XFS_ERROR(EFSCORRUPTED);
+	}
+
+
 	if (ip->i_df.if_flags & XFS_IFINLINE) {
 		memcpy(link, ip->i_df.if_u1.if_data, pathlen);
 		link[pathlen] = '\0';
-- 
1.7.6.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Carlos Maiolino]

Hey, Alex and Dave

I did both changes and sent the path again, so, hopefuly its ok now :-)

Alex, if is there anything else you need to change, please, feel free to
do. I just read your email and since you should not be online now, I did
the changes and submitted the patch again.
The discussion was nice. At least everyone survived :D

Thanks hch and sandeen by the nice discussion about this patch.


-- 
--Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Dave Chinner]

On Mon, Oct 17, 2011 at 07:05:28PM -0200, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> S_ISLNK assert, since the inode mode is checked previously in
> xfs_readlink_by_handle() and via VFS.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks good. One minor thing for consistency, but consider it:

Reviewed-by: Dave Chinner <dchinner@redhat.com>

> ---
>  fs/xfs/xfs_vnodeops.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 51fc429..9ca6676 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -123,13 +123,18 @@ xfs_readlink(
>  
>  	xfs_ilock(ip, XFS_ILOCK_SHARED);
>  
> -	ASSERT(S_ISLNK(ip->i_d.di_mode));
> -	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
> -
>  	pathlen = ip->i_d.di_size;
>  	if (!pathlen)
>  		goto out;
>  
> +	if (pathlen > MAXPATHLEN) {
> +		xfs_emerg(mp, "%s: inode (%lld) symlink length (%d) too long",
> +			 __func__, (unsigned long long)ip->i_ino, pathlen);

xfs_alert() is generally used for such messages - it's not a fatal
error (yet)....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] Fix possible memory corruption in xfs_readlink

[Alex Elder]

On Mon, 2011-10-17 at 19:05 -0200, Carlos Maiolino wrote:
> Fixes a possible memory corruption when the link is larger than
> MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
> S_ISLNK assert, since the inode mode is checked previously in
> xfs_readlink_by_handle() and via VFS.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

I know this was discussed to death on IRC.  But I didn't
get a chance to be a part of that committee so I have
a suggested change:  use %llu format, not %lld.

Just to clarify, this is addressing something that could
happen if a corrupt filesystem led to an inode whose flags
indicate it's a symlink has a size that exceeds the maximum
path length.  And without your fix, the memcpy() in
xfs_readlink() could overflow the memory it's provided.

I can implement the format string fix before I commit your
change.  But I'll wait for your permission before doing so.

Reviewed-by: Alex Elder <aelder@sgi.com>


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] Fix possible memory corruption in xfs_readlink

[Carlos Maiolino]

Fixes a possible memory corruption when the link is larger than
MAXPATHLEN and XFS_DEBUG is not enabled. This also remove the
S_ISLNK assert, since the inode mode is checked previously in
xfs_readlink_by_handle() and via VFS.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_vnodeops.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 51fc429..9ca6676 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -123,13 +123,18 @@ xfs_readlink(
 
 	xfs_ilock(ip, XFS_ILOCK_SHARED);
 
-	ASSERT(S_ISLNK(ip->i_d.di_mode));
-	ASSERT(ip->i_d.di_size <= MAXPATHLEN);
-
 	pathlen = ip->i_d.di_size;
 	if (!pathlen)
 		goto out;
 
+	if (pathlen > MAXPATHLEN) {
+		xfs_emerg(mp, "%s: inode (%lld) symlink length (%d) too long",
+			 __func__, (unsigned long long)ip->i_ino, pathlen);
+		ASSERT(0);
+		return XFS_ERROR(EFSCORRUPTED);
+	}
+
+
 	if (ip->i_df.if_flags & XFS_IFINLINE) {
 		memcpy(link, ip->i_df.if_u1.if_data, pathlen);
 		link[pathlen] = '\0';
-- 
1.7.6.2

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs