Bug 202747
| Summary: | sometime kernel crash when kzfree is called in fs/f2fs/xattr.c | ||
|---|---|---|---|
| Product: | File System | Reporter: | Jiqun Li (jiqun.li) |
| Component: | f2fs | Assignee: | Default virtual assignee for f2fs (filesystem_f2fs) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | high | CC: | chao |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | f2fs-dev | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
I didn't see many filesystems are using kzfree(), instead, there are many callers come from crypto module, I guess they use the buffer to store ciphertext or crypto key temporarily, so, before freeing those buffer it will be better to clean the data in buffer to avoid confidential data leak.
For f2fs, I think there is no such demand, and we just missed to change kzfree to kvfree in below commit:
5222595d093e ("f2fs: use kvmalloc, if kmalloc is failed")
The fixing patch has been merged, close this issue. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2a6a7e722e7a78d774ce02b847c5b183a3ff2672 |
in file fs/f2fs/xattr.c kzfree() is called somewhere example: static int read_all_xattrs(struct inode *inode, struct page *ipage, void **base_addr) { ...... txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), inline_size + size + XATTR_PADDING_SIZE, GFP_NOFS); ..... kzfree(txattr_addr); return err; } address is alloced by f2fs_kzalloc(), step into, it may use kmalloc() or kvmalloc(), accordingly the address should be freed by kfree() or kvfree(), but kzfree() aways use kfree(), then the kernel crashed when the address is alloced by kvmalloc(). I have changed kzfree to kvfree， kernel not crash any more, I not understand why set the memory to zero before free the address. so I use the patch, kernel not crash also。 it diff diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 85d9508..c4b3d7b 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -2798,6 +2798,18 @@ static inline void *f2fs_kmalloc(struct f2fs_sb_info *sbi, return kvmalloc(size, flags); } +static inline void *f2fs_kzfree(const void *p) { + size_t ks; + void *mem = (void *)p; + + if (unlikely(ZERO_OR_NULL_PTR(mem))) + return; + ks = ksize(mem); + memset(mem, 0, ks); + kvfree(mem); +} + static inline void *kvzalloc(size_t size, gfp_t flags) { void *ret; diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c index dedc91a..0152ed8 100644 --- a/fs/f2fs/xattr.c +++ b/fs/f2fs/xattr.c @@ -386,7 +386,7 @@ check: *base_addr = txattr_addr; return 0; out: - kzfree(txattr_addr); + f2fs_kzfree(txattr_addr); return err; } @@ -429,7 +429,7 @@ static int read_all_xattrs(struct inode *inode, struct page *ipage, *base_addr = txattr_addr; return 0; fail: - kzfree(txattr_addr); + f2fs_kzfree(txattr_addr); return err; } @@ -556,7 +556,7 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name, } error = size; out: - kzfree(base_addr); + f2fs_kzfree(base_addr); return error;