* [PATCH] btrfs-progs: Don't crash when processing a clone request during received
@ 2022-01-10 11:12 Nikolay Borisov
2022-01-10 11:13 ` Nikolay Borisov
2022-01-10 11:30 ` Qu Wenruo
0 siblings, 2 replies; 5+ messages in thread
From: Nikolay Borisov @ 2022-01-10 11:12 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov, Chris Muprhy
If subvol_uuid_search can't find the clone root then 'si' would either
be NULL or contain an errno. The behavior of this function was
changed as a result of commit
ac5954ee36a5 ("btrfs-progs: merge subvol_uuid_search helpers"). Before
this commit subvol_uuid_search() was a wrapper around subvol_uuid_search2
and it guaranteed to either return well-fromed 'si' or NULL. This was
sufficient for the check after the out label in process_clone.
Properly handle this new semantic by changing the simple null check to
IS_ERR_OR_NULL which covers all possible return value of
subvol_uuid_search.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
Reported-by: Chris Muprhy <lists@colorremedies.com>
Link: https://lore.kernel.org/linux-btrfs/CAJCQCtT-k3WbCSTvrvWLZQ7gLOUtTbXfOiKsGZxkPVb1g2srWg@mail.gmail.com/
---
cmds/receive.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmds/receive.c b/cmds/receive.c
index b4099bc482b2..16b9b4a853be 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -794,7 +794,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
}
out:
- if (si) {
+ if (!IS_ERR_OR_NULL(si)) {
free(si->path);
free(si);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: Don't crash when processing a clone request during received
2022-01-10 11:12 [PATCH] btrfs-progs: Don't crash when processing a clone request during received Nikolay Borisov
@ 2022-01-10 11:13 ` Nikolay Borisov
2022-01-11 16:37 ` David Sterba
2022-01-10 11:30 ` Qu Wenruo
1 sibling, 1 reply; 5+ messages in thread
From: Nikolay Borisov @ 2022-01-10 11:13 UTC (permalink / raw)
To: linux-btrfs; +Cc: Chris Muprhy
On 10.01.22 г. 13:12, Nikolay Borisov wrote:
> If subvol_uuid_search can't find the clone root then 'si' would either
> be NULL or contain an errno. The behavior of this function was
> changed as a result of commit
> ac5954ee36a5 ("btrfs-progs: merge subvol_uuid_search helpers"). Before
> this commit subvol_uuid_search() was a wrapper around subvol_uuid_search2
> and it guaranteed to either return well-fromed 'si' or NULL. This was
> sufficient for the check after the out label in process_clone.
>
> Properly handle this new semantic by changing the simple null check to
> IS_ERR_OR_NULL which covers all possible return value of
> subvol_uuid_search.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Reported-by: Chris Muprhy <lists@colorremedies.com>
> Link: https://lore.kernel.org/linux-btrfs/CAJCQCtT-k3WbCSTvrvWLZQ7gLOUtTbXfOiKsGZxkPVb1g2srWg@mail.gmail.com/
Qu pointed me to
https://patchwork.kernel.org/project/linux-btrfs/patch/20220102015016.48470-1-davispuh@gmail.com/
which is essentially the same patch. So you can drop this.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: Don't crash when processing a clone request during received
2022-01-10 11:12 [PATCH] btrfs-progs: Don't crash when processing a clone request during received Nikolay Borisov
2022-01-10 11:13 ` Nikolay Borisov
@ 2022-01-10 11:30 ` Qu Wenruo
2022-01-10 11:36 ` Nikolay Borisov
1 sibling, 1 reply; 5+ messages in thread
From: Qu Wenruo @ 2022-01-10 11:30 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs; +Cc: Chris Muprhy
On 2022/1/10 19:12, Nikolay Borisov wrote:
> If subvol_uuid_search can't find the clone root then 'si' would either
> be NULL or contain an errno. The behavior of this function was
> changed as a result of commit
> ac5954ee36a5 ("btrfs-progs: merge subvol_uuid_search helpers"). Before
> this commit subvol_uuid_search() was a wrapper around subvol_uuid_search2
> and it guaranteed to either return well-fromed 'si' or NULL. This was
> sufficient for the check after the out label in process_clone.
>
> Properly handle this new semantic by changing the simple null check to
> IS_ERR_OR_NULL which covers all possible return value of
> subvol_uuid_search.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> Reported-by: Chris Muprhy <lists@colorremedies.com>
> Link: https://lore.kernel.org/linux-btrfs/CAJCQCtT-k3WbCSTvrvWLZQ7gLOUtTbXfOiKsGZxkPVb1g2srWg@mail.gmail.com/
Reviewed-by: Qu Wenruo <wqu@suse.com>
Just curious that, can we refactor the helper to remove the NULL pointer
return case?
Thanks,
Qu
> ---
> cmds/receive.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmds/receive.c b/cmds/receive.c
> index b4099bc482b2..16b9b4a853be 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -794,7 +794,7 @@ static int process_clone(const char *path, u64 offset, u64 len,
> }
>
> out:
> - if (si) {
> + if (!IS_ERR_OR_NULL(si)) {
> free(si->path);
> free(si);
> }
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: Don't crash when processing a clone request during received
2022-01-10 11:30 ` Qu Wenruo
@ 2022-01-10 11:36 ` Nikolay Borisov
0 siblings, 0 replies; 5+ messages in thread
From: Nikolay Borisov @ 2022-01-10 11:36 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: Chris Muprhy
On 10.01.22 г. 13:30, Qu Wenruo wrote:
>
> Just curious that, can we refactor the helper to remove the NULL pointer
> return case?
Yes, the change is simple enough:
diff --git a/common/send-utils.c b/common/send-utils.c
index 4ed448b1f306..5002cfbbd0e8 100644
--- a/common/send-utils.c
+++ b/common/send-utils.c
@@ -308,10 +308,8 @@ struct subvol_info *subvol_uuid_search(int mnt_fd,
goto out;
info = calloc(1, sizeof(*info));
- if (!info) {
- ret = -ENOMEM;
- goto out;
- }
+ if (!info)
+ return ERR_PTR(-ENOMEM);
info->root_id = root_id;
memcpy(info->uuid, root_item.uuid, BTRFS_UUID_SIZE);
memcpy(info->received_uuid, root_item.received_uuid,
BTRFS_UUID_SIZE);
However this only results in callers doing IS_ERR() rather than
IS_ERR_OR_NULL imo not much of a win.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] btrfs-progs: Don't crash when processing a clone request during received
2022-01-10 11:13 ` Nikolay Borisov
@ 2022-01-11 16:37 ` David Sterba
0 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2022-01-11 16:37 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: linux-btrfs, Chris Muprhy
On Mon, Jan 10, 2022 at 01:13:46PM +0200, Nikolay Borisov wrote:
>
>
> On 10.01.22 г. 13:12, Nikolay Borisov wrote:
> > If subvol_uuid_search can't find the clone root then 'si' would either
> > be NULL or contain an errno. The behavior of this function was
> > changed as a result of commit
> > ac5954ee36a5 ("btrfs-progs: merge subvol_uuid_search helpers"). Before
> > this commit subvol_uuid_search() was a wrapper around subvol_uuid_search2
> > and it guaranteed to either return well-fromed 'si' or NULL. This was
> > sufficient for the check after the out label in process_clone.
> >
> > Properly handle this new semantic by changing the simple null check to
> > IS_ERR_OR_NULL which covers all possible return value of
> > subvol_uuid_search.
> >
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> > Reported-by: Chris Muprhy <lists@colorremedies.com>
> > Link: https://lore.kernel.org/linux-btrfs/CAJCQCtT-k3WbCSTvrvWLZQ7gLOUtTbXfOiKsGZxkPVb1g2srWg@mail.gmail.com/
>
>
> Qu pointed me to
> https://patchwork.kernel.org/project/linux-btrfs/patch/20220102015016.48470-1-davispuh@gmail.com/
>
> which is essentially the same patch. So you can drop this.
As suggested I've updated the changelog with yours as it's more
descriptive, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-11 16:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-10 11:12 [PATCH] btrfs-progs: Don't crash when processing a clone request during received Nikolay Borisov
2022-01-10 11:13 ` Nikolay Borisov
2022-01-11 16:37 ` David Sterba
2022-01-10 11:30 ` Qu Wenruo
2022-01-10 11:36 ` Nikolay Borisov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.