| Message ID | 20241009080833.1355894-1-jroi.martin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fix uninitialized pointer free on add_inode_ref | expand |
On Wed, Oct 9, 2024 at 9:17 AM Roi Martin <jroi.martin@gmail.com> wrote: > > The "add_inode_ref" function does not initializes the "name" struct > when it is declared. If any of the following calls to > "read_one_inode" returns NULL, > > dir = read_one_inode(root, parent_objectid); > if (!dir) { > ret = -ENOENT; > goto out; > } > > inode = read_one_inode(root, inode_objectid); > if (!inode) { > ret = -EIO; > goto out; > } > > then "name.name" would be freed on "out" before being initialized. > > out: > ... > kfree(name.name); > > This issue was reported by Coverity with CID 1526744. > > Signed-off-by: Roi Martin <jroi.martin@gmail.com> Fixes: e43eec81c5167b65 ("btrfs: use struct qstr instead of name and namelen pairs") Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks good, thanks. > --- > fs/btrfs/tree-log.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c > index e2ed2a791f8f..35c452bab1ca 100644 > --- a/fs/btrfs/tree-log.c > +++ b/fs/btrfs/tree-log.c > @@ -1374,7 +1374,7 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans, > struct inode *inode = NULL; > unsigned long ref_ptr; > unsigned long ref_end; > - struct fscrypt_str name; > + struct fscrypt_str name = { 0 }; > int ret; > int log_ref_ver = 0; > u64 parent_objectid; > > base-commit: 75b607fab38d149f232f01eae5e6392b394dd659 > -- > 2.46.0 > >
On Wed, Oct 09, 2024 at 10:08:33AM +0200, Roi Martin wrote: > The "add_inode_ref" function does not initializes the "name" struct > when it is declared. If any of the following calls to > "read_one_inode" returns NULL, > > dir = read_one_inode(root, parent_objectid); > if (!dir) { > ret = -ENOENT; > goto out; > } > > inode = read_one_inode(root, inode_objectid); > if (!inode) { > ret = -EIO; > goto out; > } > > then "name.name" would be freed on "out" before being initialized. > > out: > ... > kfree(name.name); > > This issue was reported by Coverity with CID 1526744. > > Signed-off-by: Roi Martin <jroi.martin@gmail.com> Reviewed-by: David Sterba <dsterba@suse.com> Added to for-next, thanks.
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index e2ed2a791f8f..35c452bab1ca 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c @@ -1374,7 +1374,7 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans, struct inode *inode = NULL; unsigned long ref_ptr; unsigned long ref_end; - struct fscrypt_str name; + struct fscrypt_str name = { 0 }; int ret; int log_ref_ver = 0; u64 parent_objectid;
The "add_inode_ref" function does not initializes the "name" struct when it is declared. If any of the following calls to "read_one_inode" returns NULL, dir = read_one_inode(root, parent_objectid); if (!dir) { ret = -ENOENT; goto out; } inode = read_one_inode(root, inode_objectid); if (!inode) { ret = -EIO; goto out; } then "name.name" would be freed on "out" before being initialized. out: ... kfree(name.name); This issue was reported by Coverity with CID 1526744. Signed-off-by: Roi Martin <jroi.martin@gmail.com> --- fs/btrfs/tree-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 75b607fab38d149f232f01eae5e6392b394dd659