[Cluster-devel] 3.18.5 kernel panic: fs/gfs2/acl.c:76

[Cluster-devel] 3.18.5 kernel panic: fs/gfs2/acl.c:76

[Andrew W Elble]

3.18.5 kernel crashing on acl deletion:

null pointer dereference in fs/gfs2/acl.c:76

to replicate:

Prereq: gfs2 filesystem w/ acl mount option turned on.

Execute:

mkdir testdir
setfacl -m d:u::rwx,d:g::rwx,d:g:wheel:rwx,d:m::rwx,d:o::--- testdir
setfattr -x system.posix_acl_default testdir

fix we're using currently:

---
 fs/gfs2/acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
index 3088e2a..8339754 100644
--- a/fs/gfs2/acl.c
+++ b/fs/gfs2/acl.c
@@ -73,7 +73,7 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 
        BUG_ON(name == NULL);
 
-       if (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode)))
+       if ((acl) && (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode))))
                return -E2BIG;
 
        if (type == ACL_TYPE_ACCESS) {
-- 
1.9.2

Thanks,

Andy

-- 
Andrew W. Elble
aweits at discipline.rit.edu
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

[Cluster-devel] 3.18.5 kernel panic: fs/gfs2/acl.c:76

[Andreas Gruenbacher]

Andrew,

> 3.18.5 kernel crashing on acl deletion:
> 
> null pointer dereference in fs/gfs2/acl.c:76

this bug seems to exist since commit 2646a1f6 from October 2009.

> fix we're using currently:
> 
> ---
>  fs/gfs2/acl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> index 3088e2a..8339754 100644
> --- a/fs/gfs2/acl.c
> +++ b/fs/gfs2/acl.c
> @@ -73,7 +73,7 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl
> *acl, int type)
>  
>         BUG_ON(name == NULL);
>  
> -       if (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode)))
> +       if ((acl) && (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode))))
>                 return -E2BIG;
>  
>         if (type == ACL_TYPE_ACCESS) {

Except for the extra parentheses this seems correct, thank you.

Andreas

[Cluster-devel] 3.18.5 kernel panic: fs/gfs2/acl.c:76

[Andrew Price]

On 06/02/15 23:50, Andreas Gruenbacher wrote:
> Andrew,
>
>> 3.18.5 kernel crashing on acl deletion:
>>
>> null pointer dereference in fs/gfs2/acl.c:76
>
> this bug seems to exist since commit 2646a1f6 from October 2009.

The if-statement originates in 2646a1f6 but the bug was introduced by 
the deletion of a NULL check in e01580bf9e which was in December 2013.

>> fix we're using currently:
>>
>> ---
>>   fs/gfs2/acl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
>> index 3088e2a..8339754 100644
>> --- a/fs/gfs2/acl.c
>> +++ b/fs/gfs2/acl.c
>> @@ -73,7 +73,7 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl
>> *acl, int type)
>>
>>          BUG_ON(name == NULL);
>>
>> -       if (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode)))
>> +       if ((acl) && (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode))))
>>                  return -E2BIG;
>>
>>          if (type == ACL_TYPE_ACCESS) {
>
> Except for the extra parentheses this seems correct, thank you.

Agreed. Good catch.

Thanks,
Andy

[Cluster-devel] 3.18.5 kernel panic: fs/gfs2/acl.c:76

[Bob Peterson]

----- Original Message -----
> On 06/02/15 23:50, Andreas Gruenbacher wrote:
> > Andrew,
> >
> >> 3.18.5 kernel crashing on acl deletion:
> >>
> >> null pointer dereference in fs/gfs2/acl.c:76
> >
> > this bug seems to exist since commit 2646a1f6 from October 2009.
> 
> The if-statement originates in 2646a1f6 but the bug was introduced by
> the deletion of a NULL check in e01580bf9e which was in December 2013.
> 
> >> fix we're using currently:
> >>
> >> ---
> >>   fs/gfs2/acl.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
> >> index 3088e2a..8339754 100644
> >> --- a/fs/gfs2/acl.c
> >> +++ b/fs/gfs2/acl.c
> >> @@ -73,7 +73,7 @@ int gfs2_set_acl(struct inode *inode, struct posix_acl
> >> *acl, int type)
> >>
> >>          BUG_ON(name == NULL);
> >>
> >> -       if (acl->a_count > GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode)))
> >> +       if ((acl) && (acl->a_count >
> >> GFS2_ACL_MAX_ENTRIES(GFS2_SB(inode))))
> >>                  return -E2BIG;
> >>
> >>          if (type == ACL_TYPE_ACCESS) {
> >
> > Except for the extra parentheses this seems correct, thank you.
> 
> Agreed. Good catch.
> 
> Thanks,
> Andy
Hi,

Christoph's patch, which introduced the problem, was never ported to RHEL7, so
let's just treat this as an upstream bug.

Andreas: I think maybe you should post your acl patch separately.

Andrew Elble:
I don't think we even need a bugzilla for this one. Do you want to just
post your latest patch (with fewer parentheses) to cluster-devel at redhat.com
so Steve Whitehouse can pick it up in the GFS2 nmw git tree? Then you can
get the credit.

Regards,

Bob Peterson
Red Hat File Systems