Hi Aurélien, On Thu, 05 Jul 2018 13:30:43 +0200 Aurélien Aptel <aaptel@xxxxxxxx> wrote: > Hi Stefano, > > Good catch! But I think this bug hides a bigger more sinister bug... > > Stefano Brivio <sbrivio@xxxxxxxxxx> writes: > > - buf->lcontext.LeaseKeyLow = cpu_to_le64(*((u64 *)lease_key)); > > - buf->lcontext.LeaseKeyHigh = cpu_to_le64(*((u64 *)(lease_key + 8))); > > + get_random_bytes(&buf->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE); > > buf->lcontext.LeaseState = map_oplock_to_lease(oplock); > > I know the value used to be kind of random and changing it to actual > random bytes should be the same however we already have a function to > generate a lease key in the generic version operations: > > /* generate new lease key */ > void (*new_lease_key)(struct cifs_fid *); > > Which are used to set the lease key in a struct cifs_fid and are called > on both create & open code paths (in cifs_create and cifs_atomic_open). > > I think the code is assuming being called by smb2_open_file(): > > __u8 smb2_oplock[17]; <--- space for oplock type + key buffer > ... > *smb2_oplock = SMB2_OPLOCK_LEVEL_BATCH; > ... > if (oparms->tcon->ses->server->capabilities & SMB2_GLOBAL_CAP_LEASING) > memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE); > ... > rc = SMB2_open(xid, oparms, smb2_path, smb2_oplock, smb2_data, NULL, > NULL); > > So it seems all SMB2_open() callers that request an oplock MUST allocate this 17 bytes > buffer and from a quick look many of them don't. Aha! I didn't spot that. Everything makes sense now. Thanks for the explanation! > The proper fix would be to pass the fid->lease_key when creating the context. > > Hope that was helpful enough for a v2 ;) Indeed, that was extremely helpful. Preparing v2 now. -- Stefano -- 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
