Messages in this thread Patch in this message | | | Subject | Re: [PATCH 1/2] ubifs: xattr: Fix some potential memory leaks while iterating entries | From | Zhihao Cheng <> | Date | Mon, 14 Sep 2020 11:24:55 +0800 |
| |
在 2020/9/14 3:08, Richard Weinberger 写道: > On Mon, Jun 1, 2020 at 11:11 AM Zhihao Cheng <chengzhihao1@huawei.com> wrote: >>
> I agree that this needs fixing. Did you also look into getting rid of pxent? > UBIFS uses the pxent pattern over and over and the same error got copy pasted > a lot. :-( > I thought about it. I'm not sure whether it is good to drop 'pxent'. Maybe you mean doing changes looks like following(Takes ubifs_jnl_write_inode() for example):
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c index 4a5b06f8d812..fcd5ac047b34 100644 --- a/fs/ubifs/journal.c +++ b/fs/ubifs/journal.c @@ -879,13 +879,19 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) union ubifs_key key; struct fscrypt_name nm = {0}; struct inode *xino; - struct ubifs_dent_node *xent, *pxent = NULL; + struct ubifs_dent_node *xent;
if (ui->xattr_cnt >= ubifs_xattr_max_cnt(c)) { ubifs_err(c, "Cannot delete inode, it has too much xattrs!"); goto out_release; }
+ fname_name(&nm) = kmalloc(UBIFS_MAX_NLEN, GFP_NOFS); + if (!fname_name(&nm)) { + err = -ENOMEM; + goto out_release; + } + lowest_xent_key(c, &key, inode->i_ino); while (1) { xent = ubifs_tnc_next_ent(c, &key, &nm); @@ -894,11 +900,12 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) if (err == -ENOENT) break;
+ kfree(fname_name(&nm)); goto out_release; }
- fname_name(&nm) = xent->name; fname_len(&nm) = le16_to_cpu(xent->nlen); + strncpy(fname_name(&nm), xent->name, fname_len(&nm));
xino = ubifs_iget(c->vfs_sb, le64_to_cpu(xent->inum)); if (IS_ERR(xino)) { @@ -907,6 +914,7 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) xent->name, err); ubifs_ro_mode(c, err); kfree(xent); + kfree(fname_name(&nm)); goto out_release; } ubifs_assert(c, ubifs_inode(xino)->xattr); @@ -916,11 +924,10 @@ int ubifs_jnl_write_inode(struct ubifs_info *c, const struct inode *inode) ino = (void *)ino + UBIFS_INO_NODE_SZ; iput(xino);
- kfree(pxent); - pxent = xent; key_read(c, &xent->key, &key); + kfree(xent); } - kfree(pxent); + kfree(fname_name(&nm)); }
pack_inode(c, ino, inode, 1); The kill_xattrs process is more intuitive without the pxent. However, the release process for the memory (stores xent->name) is similar to 'pxent'. If you think it's better than v1, I will send v2.
| |