linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* unused fs/jffs2/acl.c:jffs2_clear_acl()
@ 2006-06-29 13:01 Adrian Bunk
  2006-06-29 13:16 ` David Woodhouse
  0 siblings, 1 reply; 4+ messages in thread
From: Adrian Bunk @ 2006-06-29 13:01 UTC (permalink / raw)
  To: dwmw2; +Cc: jffs-dev, linux-kernel

Hi David,

it might not have been intended that jffs2_clear_acl() in Linus' tree is 
unused?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: unused fs/jffs2/acl.c:jffs2_clear_acl()
  2006-06-29 13:01 unused fs/jffs2/acl.c:jffs2_clear_acl() Adrian Bunk
@ 2006-06-29 13:16 ` David Woodhouse
  2006-06-29 14:27   ` KaiGai Kohei
  0 siblings, 1 reply; 4+ messages in thread
From: David Woodhouse @ 2006-06-29 13:16 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: jffs-dev, linux-kernel, KaiGai Kohei

On Thu, 2006-06-29 at 15:01 +0200, Adrian Bunk wrote:
> it might not have been intended that jffs2_clear_acl() in Linus' tree
> is unused?

I suspect you're right -- thanks for pointing it out.

Kaigai-san?

-- 
dwmw2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: unused fs/jffs2/acl.c:jffs2_clear_acl()
  2006-06-29 13:16 ` David Woodhouse
@ 2006-06-29 14:27   ` KaiGai Kohei
  2006-07-02 14:08     ` KaiGai Kohei
  0 siblings, 1 reply; 4+ messages in thread
From: KaiGai Kohei @ 2006-06-29 14:27 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Adrian Bunk, jffs-dev, linux-kernel

David Woodhouse wrote:
> On Thu, 2006-06-29 at 15:01 +0200, Adrian Bunk wrote:
> 
>>it might not have been intended that jffs2_clear_acl() in Linus' tree
>>is unused?
> 
> 
> I suspect you're right -- thanks for pointing it out.
> 
> Kaigai-san?

I'm sorry, it's a serious BUG.
When an inode is cleared, jffs2_clear_acl() should be called
to release on-memory ACL. Because the current implementation
didn't care about this cleaning-up, we have memory-leaking.

Please wait a patch for a while.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: unused fs/jffs2/acl.c:jffs2_clear_acl()
  2006-06-29 14:27   ` KaiGai Kohei
@ 2006-07-02 14:08     ` KaiGai Kohei
  0 siblings, 0 replies; 4+ messages in thread
From: KaiGai Kohei @ 2006-07-02 14:08 UTC (permalink / raw)
  To: David Woodhouse; +Cc: KaiGai Kohei, Adrian Bunk, jffs-dev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1034 bytes --]

[JFFS2][XATTR] Fix memory leak in POSIX-ACL support

* jffs2-xattr-v6.2-02-fix-posix_acl-memory-leak.patch

jffs2_clear_acl() which releases acl caches allocated by kmalloc()
was defined but it was never called. Thus, we faced to the risk
of memory leaking.

This patch plugs jffs2_clear_acl() into jffs2_do_clear_inode().
It ensures to release acl cache when inode is cleared.

Signed-off-by: KaiGai Kohei <kaigai@ak.jp.nec.com>

Thanks,

KaiGai Kohei wrote:
> David Woodhouse wrote:
> 
>>On Thu, 2006-06-29 at 15:01 +0200, Adrian Bunk wrote:
>>
>>
>>>it might not have been intended that jffs2_clear_acl() in Linus' tree
>>>is unused?
>>
>>
>>I suspect you're right -- thanks for pointing it out.
>>
>>Kaigai-san?
> 
> 
> I'm sorry, it's a serious BUG.
> When an inode is cleared, jffs2_clear_acl() should be called
> to release on-memory ACL. Because the current implementation
> didn't care about this cleaning-up, we have memory-leaking.
> 
> Please wait a patch for a while.
> 
> Thanks,

-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

[-- Attachment #2: jffs2-xattr-v6.2-02-fix-posix_acl-memory-leak.patch --]
[-- Type: text/plain, Size: 1702 bytes --]

diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
index 9c2077e..0ae3cd1 100644
--- a/fs/jffs2/acl.c
+++ b/fs/jffs2/acl.c
@@ -345,10 +345,8 @@ int jffs2_init_acl(struct inode *inode, 
 	return rc;
 }
 
-void jffs2_clear_acl(struct inode *inode)
+void jffs2_clear_acl(struct jffs2_inode_info *f)
 {
-	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
-
 	if (f->i_acl_access && f->i_acl_access != JFFS2_ACL_NOT_CACHED) {
 		posix_acl_release(f->i_acl_access);
 		f->i_acl_access = JFFS2_ACL_NOT_CACHED;
diff --git a/fs/jffs2/acl.h b/fs/jffs2/acl.h
index 8893bd1..fa327db 100644
--- a/fs/jffs2/acl.h
+++ b/fs/jffs2/acl.h
@@ -30,7 +30,7 @@ #define JFFS2_ACL_NOT_CACHED ((void *)-1
 extern int jffs2_permission(struct inode *, int, struct nameidata *);
 extern int jffs2_acl_chmod(struct inode *);
 extern int jffs2_init_acl(struct inode *, struct inode *);
-extern void jffs2_clear_acl(struct inode *);
+extern void jffs2_clear_acl(struct jffs2_inode_info *);
 
 extern struct xattr_handler jffs2_acl_access_xattr_handler;
 extern struct xattr_handler jffs2_acl_default_xattr_handler;
@@ -40,6 +40,6 @@ #else
 #define jffs2_permission NULL
 #define jffs2_acl_chmod(inode)		(0)
 #define jffs2_init_acl(inode,dir)	(0)
-#define jffs2_clear_acl(inode)
+#define jffs2_clear_acl(f)
 
 #endif	/* CONFIG_JFFS2_FS_POSIX_ACL */
diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
index cc18992..266423b 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -968,6 +968,7 @@ void jffs2_do_clear_inode(struct jffs2_s
 	struct jffs2_full_dirent *fd, *fds;
 	int deleted;
 
+	jffs2_clear_acl(f);
 	jffs2_xattr_delete_inode(c, f->inocache);
 	down(&f->sem);
 	deleted = f->inocache && !f->inocache->nlink;

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2006-07-02 14:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-06-29 13:01 unused fs/jffs2/acl.c:jffs2_clear_acl() Adrian Bunk
2006-06-29 13:16 ` David Woodhouse
2006-06-29 14:27   ` KaiGai Kohei
2006-07-02 14:08     ` KaiGai Kohei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).