| Message ID | d915ceb4d459aff89c0264113db21592a6806db1.1652517184.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Fix an error handling path in btrfs_read_sys_array() | expand |
On 2022/5/14 20:01, Christophe JAILLET wrote: > If alloc_dummy_extent_buffer() we should return an error code, not 0 that > would mean success. > > Fixes: a1fc41ac28d3 ("btrfs: use dummy extent buffer for super block sys chunk array read") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Reviewed-by: Qu Wenruo <wqu@suse.com> All my fault, thanks for catching it. Qu > --- > fs/btrfs/volumes.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index b2d5a54ea172..9c20049d1fec 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -7359,7 +7359,7 @@ int btrfs_read_sys_array(struct btrfs_fs_info *fs_info) > */ > sb = alloc_dummy_extent_buffer(fs_info, BTRFS_SUPER_INFO_OFFSET); > if (!sb) > - return PTR_ERR(sb); > + return -ENOMEM; > set_extent_buffer_uptodate(sb); > > write_extent_buffer(sb, super_copy, 0, BTRFS_SUPER_INFO_SIZE);
On Sun, May 15, 2022 at 06:57:25AM +0800, Qu Wenruo wrote: > > > On 2022/5/14 20:01, Christophe JAILLET wrote: > > If alloc_dummy_extent_buffer() we should return an error code, not 0 that > > would mean success. > > > > Fixes: a1fc41ac28d3 ("btrfs: use dummy extent buffer for super block sys chunk array read") > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > Reviewed-by: Qu Wenruo <wqu@suse.com> > > All my fault, thanks for catching it. > Qu > I sent this patch in January and David was going to fold it into the original patch but it got lost. Thanks, Christophe! regards, dan carpenter
On Mon, May 16, 2022 at 04:54:07PM +0300, Dan Carpenter wrote: > On Sun, May 15, 2022 at 06:57:25AM +0800, Qu Wenruo wrote: > > > > > > On 2022/5/14 20:01, Christophe JAILLET wrote: > > > If alloc_dummy_extent_buffer() we should return an error code, not 0 that > > > would mean success. > > > > > > Fixes: a1fc41ac28d3 ("btrfs: use dummy extent buffer for super block sys chunk array read") > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > > Reviewed-by: Qu Wenruo <wqu@suse.com> > > > > All my fault, thanks for catching it. > > Qu > > > > I sent this patch in January and David was going to fold it into the > original patch but it got lost. Thanks, Christophe! I found my reply to your fix that I folded the fixup, but then it got lost for some reason. Probably because I picked the patchset from mailing list again and did not take the local branch. I'll fold the fix again as it' still in the unmerged branch. Thanks.
Le 16/05/2022 à 17:01, David Sterba a écrit : > On Mon, May 16, 2022 at 04:54:07PM +0300, Dan Carpenter wrote: >> On Sun, May 15, 2022 at 06:57:25AM +0800, Qu Wenruo wrote: >>> >>> >>> On 2022/5/14 20:01, Christophe JAILLET wrote: >>>> If alloc_dummy_extent_buffer() we should return an error code, not 0 that >>>> would mean success. >>>> >>>> Fixes: a1fc41ac28d3 ("btrfs: use dummy extent buffer for super block sys chunk array read") >>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >>> >>> Reviewed-by: Qu Wenruo <wqu@suse.com> >>> >>> All my fault, thanks for catching it. >>> Qu >>> >> >> I sent this patch in January and David was going to fold it into the >> original patch but it got lost. Thanks, Christophe! Hi, Not exactly. Your patch was: - if (IS_ERR(sb)) - return PTR_ERR(sb); + if (!sb) + return -ENOMEM; Mine is only: - return PTR_ERR(sb); + return -ENOMEM; So for some reason, what you had reported was just half applied. (or half fixed by someone else) > > I found my reply to your fix that I folded the fixup, but then it got > lost for some reason. Probably because I picked the patchset from > mailing list again and did not take the local branch. I'll fold the fix > again as it' still in the unmerged branch. Thanks. > just in case, the Fixes tag in Dan's patch leads to: "Notice: this object is not reachable from any branch." I don't think that it is of any importance if the fix in folded, but in case, I let you know. CJ
On Mon, May 16, 2022 at 07:42:23PM +0200, Christophe JAILLET wrote: > Le 16/05/2022 à 17:01, David Sterba a écrit : > > On Mon, May 16, 2022 at 04:54:07PM +0300, Dan Carpenter wrote: > > > On Sun, May 15, 2022 at 06:57:25AM +0800, Qu Wenruo wrote: > > > > > > > > > > > > On 2022/5/14 20:01, Christophe JAILLET wrote: > > > > > If alloc_dummy_extent_buffer() we should return an error code, not 0 that > > > > > would mean success. > > > > > > > > > > Fixes: a1fc41ac28d3 ("btrfs: use dummy extent buffer for super block sys chunk array read") > > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > > > > > > Reviewed-by: Qu Wenruo <wqu@suse.com> > > > > > > > > All my fault, thanks for catching it. > > > > Qu > > > > > > > > > > I sent this patch in January and David was going to fold it into the > > > original patch but it got lost. Thanks, Christophe! > > Hi, > > Not exactly. > Your patch was: > - if (IS_ERR(sb)) > - return PTR_ERR(sb); > + if (!sb) > + return -ENOMEM; > > Mine is only: > - return PTR_ERR(sb); > + return -ENOMEM; > > So for some reason, what you had reported was just half applied. (or half > fixed by someone else) > Oh that's interesting. I must have seen the same Smatch warning that you saw and ignored it because I got confused which patch I had sent. The kbuild-bot also tried to send a warning, but I squashed it for the same reasons. So in terms of process issues and avoidable bugs this one is partly my bad. regards, dan carpenter
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index b2d5a54ea172..9c20049d1fec 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -7359,7 +7359,7 @@ int btrfs_read_sys_array(struct btrfs_fs_info *fs_info) */ sb = alloc_dummy_extent_buffer(fs_info, BTRFS_SUPER_INFO_OFFSET); if (!sb) - return PTR_ERR(sb); + return -ENOMEM; set_extent_buffer_uptodate(sb); write_extent_buffer(sb, super_copy, 0, BTRFS_SUPER_INFO_SIZE);
If alloc_dummy_extent_buffer() we should return an error code, not 0 that would mean success. Fixes: a1fc41ac28d3 ("btrfs: use dummy extent buffer for super block sys chunk array read") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- fs/btrfs/volumes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)