[Virtio-fs] [PATCH] virtiofsd: fix double fuse_mbuf_iter_advance when do_removemapping
Peng Tao
tao.peng at linux.alibaba.com
Thu Dec 5 10:04:34 UTC 2019
On 2019/12/5 17:34, Catherine Ho wrote:
> Hi Peng
>
> On Thu, 5 Dec 2019 at 17:19, Peng Tao <tao.peng at linux.alibaba.com> wrote:
>>
>> On 2019/12/5 17:01, Catherine Ho wrote:
>>> Commit 8e92b1fc98f7 ("DAX: virtiofsd: make FUSE_REMOVEMAPPING support
>>> multiple entries") forgot to remove one fuse_mbuf_iter_advance in
>>> do_removemapping.
>> No, we do need to advance twice. One for fuse_removemapping_in, another
>> for an array of fuse_removemapping_one.
>>
> But seems it advances for 3 times. What I mean is the 2nd advance of *one*
> is pointless.
> Please see [1] [2]
> [1]https://gitlab.com/virtio-fs/qemu/blob/virtio-fs-dev/contrib/virtiofsd/fuse_lowlevel.c#L1888
> [2]https://gitlab.com/virtio-fs/qemu/blob/virtio-fs-dev/contrib/virtiofsd/fuse_lowlevel.c#L1896
>
oops, that does look like a merge hunk. I don't have it in my local
branch. After pulling the latest code I can see it too. And there was a
branch reset at some point.
From https://gitlab.com/virtio-fs/qemu
+ 42db960...5a356e6 virtio-fs-dev -> virtiofs/virtio-fs-dev (forced
update)
Now your patch looks good to me. Thanks!
Cheers,
Tao
> Best Regards
> Catherine
>>>
>>> Without this patch, virtiofsd will report:
>>> [ID: 00000123] do_removemapping: invalid in, expected 1 * 16, has 60 - 60
>>> [ID: 00000123] unique: 232, error: -22 (Invalid argument), outsize: 16
>>>
>> What kernel version are you using? It appears that the remove mapping
>> request does not container a proper fuse_removemapping_one struct.
>>
>>> Fixes: 8e92b1fc98f7 ("DAX: virtiofsd: make FUSE_REMOVEMAPPING support multiple entries")
>>> Cc: Peng Tao <tao.peng at linux.alibaba.com>
>>> Signed-off-by: Catherine Ho <catherine.hecx at gmail.com>
Reviewed-by: Peng Tao <tao.peng at linux.alibaba.com>
>>> ---
>>> contrib/virtiofsd/fuse_lowlevel.c | 8 --------
>>> 1 file changed, 8 deletions(-)
>>>
>>> diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
>>> index bde66c72c3..752e7d97b9 100644
>>> --- a/contrib/virtiofsd/fuse_lowlevel.c
>>> +++ b/contrib/virtiofsd/fuse_lowlevel.c
>>> @@ -1894,14 +1894,6 @@ static void do_removemapping(fuse_req_t req, fuse_ino_t nodeid,
>>> return;
>>> }
>>>
>>> - one = fuse_mbuf_iter_advance(iter, sizeof(*one));
>>> - if (!one) {
>>> - fuse_log(FUSE_LOG_ERR, "do_removemapping: invalid in, expected %d * %ld, has %ld - %ld\n",
>>> - arg->count, sizeof(*one), iter->size, iter->pos);
>>> - fuse_reply_err(req, EINVAL);
>>> - return;
>>> - }
>>> -
>>> if (req->se->op.removemapping)
>>> req->se->op.removemapping(req, req->se, nodeid, arg->count, one);
>> The patch is wrong itself, as `one` is not assigned here.
>>
--
Into something rich and strange.
More information about the Virtio-fs
mailing list