2018-07-27 5:26 GMT-07:00 Stefano Brivio <sbrivio@xxxxxxxxxx>:
> Hi Pavel,
>
> On Thu, 26 Jul 2018 15:30:32 -0700
> Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
>> > Suggested-by: Aurélien Aptel <aaptel@xxxxxxxx>
>> > Fixes: b8c32dbb0deb ("CIFS: Request SMB2.1 leases")
>> > Signed-off-by: Stefano Brivio <sbrivio@xxxxxxxxxx>
>>
>> The commit listed above is not correct. The one that introduced the
>> problem is a93864d93977b99bda6c348a09b90a3d7ef8db3a
>> (https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commitdiff;h=a93864d93977b99bda6c348a09b90a3d7ef8db3a;hp=2fbb56446fde14a80790de9b182ae6f7c36a039a)
>> was merged recently, so, there is no need to push to stable kernels as
>> it might be seemed previously looking at the problematic commit.
>
Hi Stefano,
> Maybe I'm missing something, but commit b8c32dbb0deb ("CIFS: Request
> SMB2.1 leases") introduces:
>
> buf->lcontext.LeaseKeyLow = cpu_to_le64(*((u64 *)lease_key));
> buf->lcontext.LeaseKeyHigh = cpu_to_le64(*((u64 *)(lease_key + 8)));
>
> in create_lease_buf(). If we reach that coming from smb2_open_file(),
> it's fine, but if we're coming from other callers of SMB2_open() (see e.g.
> smb2_query_dir_first() at b8c32dbb0deb) we hit the same issue, don't we?
create_lease_buf() is called by add_lease_context() which is called if
(server->capabilities & SMB2_GLOBAL_CAP_LEASING) and oplock is not
NONE. smb2_query_dir_first() sets oplock level to NONE, so is not
affected by this issue. Commit
a93864d93977b99bda6c348a09b90a3d7ef8db3a changed a lease level to
LEVEL_II that's why you caught the issue.
Actually, when I grep'ed for "SMB2_OPLOCK_LEVEL" in the code I noticed
that the problem existed even before: if we mount with mfsymlink
option we should hit the same bug (since kernel v3.18). Thus, the
proper fix needs to be pushed to stable but the current patch relies
on fid->lease_key which was added very recently.
I would say that we need another patch for that: we don't need to
request a lease in mfsymlink code, so we can just change
smb3_create_mf_symlink() and smb3_query_mf_symlink() to use
SMB2_OPLOCK_LEVEL_NONE instead. This can be done in both mainline and
stable trees, so, we can merge it easily.
Anyway, thanks for fixing the problem and cleaning up the code - now
it looks more intuitive and the behavior is predictable!
--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
