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

2015-02-09 Thread 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

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

2015-02-09 Thread 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@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

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

2015-02-06 Thread 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
awe...@discipline.rit.edu
Infrastructure Engineer, Communications Technical Lead
Rochester Institute of Technology
PGP: BFAD 8461 4CCF DC95 DA2C B0EB 965B 082E 863E C912

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

2015-02-06 Thread 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