Re: [PATCH] cifs: check if mid was deleted in async read callback

[Enzo Matsumiya <ematsumiya@suse.de>]

On 09/19, Steve French wrote:
>merged into cifs-2.6.git for-next

Steve, please drop this one, I was wrong :(

I'll send a new patch later today.

>
>On Mon, Sep 19, 2022 at 9:43 AM Paulo Alcantara <pc@cjr.nz> wrote:
>>
>> Enzo Matsumiya <ematsumiya@suse.de> writes:
>>
>> > There's a race when cifs_readv_receive() might dequeue the mid,
>> > and mid->callback(), called from demultiplex thread, will try to
>> > access it to verify the signature before the mid is actually
>> > released/deleted.
>> >
>> > Currently the signature verification fails, but the verification
>> > shouldn't have happened at all because the mid was deleted because
>> > of an error, and hence not really supposed to be passed to
>> > ->callback(). There are no further errors because the mid is
>> > effectivelly gone by the end of the callback.
>> >
>> > This patch checks if the mid doesn't have the MID_DELETED flag set (by
>> > dequeue_mid()) right before trying to verify the signature. According to
>> > my tests, trying to check it earlier, e.g. after the ->receive() call in
>> > cifs_demultiplex_thread, will fail most of the time as dequeue_mid()
>> > might not have been called yet.
>> >
>> > This behaviour can be seen in xfstests generic/465, for example, where
>> > mids with STATUS_END_OF_FILE (-ENODATA) are dequeued and supposed to be
>> > discarded, but instead have their signature computed, but mismatched.
>> >
>> > Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
>> > ---
>> >  fs/cifs/cifssmb.c | 2 +-
>> >  fs/cifs/smb2pdu.c | 2 +-
>> >  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> Good catch!
>>
>> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
>
>
>
>-- 
>Thanks,
>
>Steve