diff mbox

[fixed] Prevent oopsing in posix_acl_valid()

Message ID 1304477648-14952-1-git-send-email-daniel.blueman@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel J Blueman May 4, 2011, 2:54 a.m. UTC
If posix_acl_from_xattr() returns an error code, a negative address is
dereferenced causing an oops; fix by checking for an error code first.

Typo fixed; too much late-night coding.

Signed-off-by: Daniel J Blueman <daniel.blueman@gmail.com>
---
 fs/btrfs/acl.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Comments

Josef Bacik May 4, 2011, 2:40 p.m. UTC | #1
On 05/03/2011 10:54 PM, Daniel J Blueman wrote:
> If posix_acl_from_xattr() returns an error code, a negative address is
> dereferenced causing an oops; fix by checking for an error code first.
>
> Typo fixed; too much late-night coding.
>
> Signed-off-by: Daniel J Blueman<daniel.blueman@gmail.com>
> ---
>   fs/btrfs/acl.c |    5 +++--
>   1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> index 5d505aa..44ea5b9 100644
> --- a/fs/btrfs/acl.c
> +++ b/fs/btrfs/acl.c
> @@ -178,12 +178,13 @@ static int btrfs_xattr_acl_set(struct dentry *dentry, const char *name,
>
>   	if (value) {
>   		acl = posix_acl_from_xattr(value, size);
> +		if (IS_ERR(acl))
> +			return PTR_ERR(acl);
> +
>   		if (acl) {
>   			ret = posix_acl_valid(acl);
>   			if (ret)
>   				goto out;
> -		} else if (IS_ERR(acl)) {
> -			return PTR_ERR(acl);
>   		}
>   	}
>

Actually pulled this down and compiled it this time to make sure it 
worked.  You can add

Reviewed-by: Josef Bacik <josef@redhat.com>

Thanks,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel J Blueman May 10, 2011, 5:42 a.m. UTC | #2
Hi Chris,

On 4 May 2011 22:40, Josef Bacik <josef@redhat.com> wrote:
> On 05/03/2011 10:54 PM, Daniel J Blueman wrote:
>>
>> If posix_acl_from_xattr() returns an error code, a negative address is
>> dereferenced causing an oops; fix by checking for an error code first.
>>
>> Typo fixed; too much late-night coding.
>>
>> Signed-off-by: Daniel J Blueman<daniel.blueman@gmail.com>
>> ---
>>  fs/btrfs/acl.c |    5 +++--
>>  1 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
>> index 5d505aa..44ea5b9 100644
>> --- a/fs/btrfs/acl.c
>> +++ b/fs/btrfs/acl.c
>> @@ -178,12 +178,13 @@ static int btrfs_xattr_acl_set(struct dentry
>> *dentry, const char *name,
>>
>>        if (value) {
>>                acl = posix_acl_from_xattr(value, size);
>> +               if (IS_ERR(acl))
>> +                       return PTR_ERR(acl);
>> +
>>                if (acl) {
>>                        ret = posix_acl_valid(acl);
>>                        if (ret)
>>                                goto out;
>> -               } else if (IS_ERR(acl)) {
>> -                       return PTR_ERR(acl);
>>                }
>>        }
>>
>
> Actually pulled this down and compiled it this time to make sure it worked.
>  You can add
>
> Reviewed-by: Josef Bacik <josef@redhat.com>

Will this fix go upstream for the final 2.6.39, now that the last -rc
is already out? I hit it in two independent cases when rebooting after
other kernel crashes.

Thanks,
  Daniel
Chris Mason May 10, 2011, 11:21 a.m. UTC | #3
Excerpts from Daniel J Blueman's message of 2011-05-10 01:42:45 -0400:
> Hi Chris,
> 
> On 4 May 2011 22:40, Josef Bacik <josef@redhat.com> wrote:
> > On 05/03/2011 10:54 PM, Daniel J Blueman wrote:
> >>
> >> If posix_acl_from_xattr() returns an error code, a negative address is
> >> dereferenced causing an oops; fix by checking for an error code first.
> >>
> >> Typo fixed; too much late-night coding.
> >>
> >> Signed-off-by: Daniel J Blueman<daniel.blueman@gmail.com>
> >> ---
> >>  fs/btrfs/acl.c |    5 +++--
> >>  1 files changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
> >> index 5d505aa..44ea5b9 100644
> >> --- a/fs/btrfs/acl.c
> >> +++ b/fs/btrfs/acl.c
> >> @@ -178,12 +178,13 @@ static int btrfs_xattr_acl_set(struct dentry
> >> *dentry, const char *name,
> >>
> >>        if (value) {
> >>                acl = posix_acl_from_xattr(value, size);
> >> +               if (IS_ERR(acl))
> >> +                       return PTR_ERR(acl);
> >> +
> >>                if (acl) {
> >>                        ret = posix_acl_valid(acl);
> >>                        if (ret)
> >>                                goto out;
> >> -               } else if (IS_ERR(acl)) {
> >> -                       return PTR_ERR(acl);
> >>                }
> >>        }
> >>
> >
> > Actually pulled this down and compiled it this time to make sure it worked.
> >  You can add
> >
> > Reviewed-by: Josef Bacik <josef@redhat.com>
> 
> Will this fix go upstream for the final 2.6.39, now that the last -rc
> is already out? I hit it in two independent cases when rebooting after
> other kernel crashes.

Yes, I have one other patch to from Li Zefan that I will send along.

-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index 5d505aa..44ea5b9 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -178,12 +178,13 @@  static int btrfs_xattr_acl_set(struct dentry *dentry, const char *name,
 
 	if (value) {
 		acl = posix_acl_from_xattr(value, size);
+		if (IS_ERR(acl))
+			return PTR_ERR(acl);
+
 		if (acl) {
 			ret = posix_acl_valid(acl);
 			if (ret)
 				goto out;
-		} else if (IS_ERR(acl)) {
-			return PTR_ERR(acl);
 		}
 	}