Re: [PATCH v3 11/11] smb: client: fix NameOffset and Next field validation in smb2_parse_contexts()

[Frank Sorenson <sorenson@redhat.com>]

On 8/29/26 9:01 AM, Namjae Jeon wrote:
>>                  case 16:
>> -                       if (posix && !memcmp(name, smb3_create_tag_posix, 16))
>> +                       if (dlen && posix && !memcmp(name, smb3_create_tag_posix, 16))
> Could we check that dlen is large enough for the fixed POSIX fields
> before calling parse_posix_ctxt()? It reads 12 bytes even when
> DataLength is smaller.

so this needs to be:
                      if (dlen > 12 && posix && !memcmp(name, smb3_create_tag_posix, 16))

I think we'll need similar before the calls to the 
server->ops->parse_lease_buf:
                     if (dlen && !strncmp(name, 
SMB2_CREATE_REQUEST_LEASE, 4)) {                                   
*oplock = server->ops->parse_lease_buf(cc, epoch,                       
                                                    lease_key);

for smb2: smb2_parse_lease_buf(void *buf, __u16 *epoch, char *lease_key) 
{         struct create_lease *lc = (struct create_lease *)buf;         
*epoch = 0; /* not used */         if (lc->lcontext.LeaseFlags & 
SMB2_LEASE_FLAG_BREAK_IN_PROGRESS_LE)                 return 
SMB2_OPLOCK_LEVEL_NOCHANGE;         return 
le32_to_cpu(lc->lcontext.LeaseState);

/* See MS-SMB2 2.2.13.2.8 */ struct lease_context {         __u8 
LeaseKey[SMB2_LEASE_KEY_SIZE];         __le32 LeaseState;         __le32 
LeaseFlags;         __le64 LeaseDuration; } __packed;

for smb2, we only access through LeaseFlags (24 bytes), but the entire 
struct is 32

static __u8 smb3_parse_lease_buf(void *buf, __u16 *epoch, char 
*lease_key) {         struct create_lease_v2 *lc = (struct 
create_lease_v2 *)buf;         *epoch = le16_to_cpu(lc->lcontext.Epoch); 
         if (lc->lcontext.LeaseFlags & 
SMB2_LEASE_FLAG_BREAK_IN_PROGRESS_LE)                 return 
SMB2_OPLOCK_LEVEL_NOCHANGE;         if (lease_key)                 
memcpy(lease_key, &lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);         
return le32_to_cpu(lc->lcontext.LeaseState);

/* See MS-SMB2 2.2.13.2.10 */ struct lease_context_v2 {         __u8 
LeaseKey[SMB2_LEASE_KEY_SIZE];         __le32 LeaseState;         __le32 
LeaseFlags;         __le64 LeaseDuration;         __u8 
ParentLeaseKey[SMB2_LEASE_KEY_SIZE];         __le16 Epoch;         
__le16 Reserved; } __packed; and for smb3, we'll need through Epoch, so

     SMB2_LEASE_KEY_SIZE + 4 + 4 + 8 + SMB2_LEASE_KEY_SIZE + 2 = 50,

with the entire struct being 52 bytes

and also before the call to parse_query_id_ctxt():
                           } else if (dlen > 16 && buf &&               
                        !strncmp(name, SMB2_CREATE_QUERY_ON_DISK_ID, 4)) 
{                                   parse_query_id_ctxt(cc, buf);
parse_query_id_ctxt(struct create_context *cc, struct smb2_file_all_info *buf)
{
         struct create_disk_id_rsp *pdisk_id = (struct create_disk_id_rsp *)cc;

         cifs_dbg(FYI, "parse query id context 0x%llx 0x%llx\n",
                 pdisk_id->DiskFileId, pdisk_id->VolumeId);
         buf->IndexNumber = pdisk_id->DiskFileId;

/* See MS-SMB2 2.2.14.2.9 */
struct create_disk_id_rsp {
         struct create_context_hdr ccontext;
         __u8   Name[8];
         __le64 DiskFileId;
         __le64 VolumeId;
         __u8  Reserved[16];
} __packed;


With varying sizes, how do we handle the check prior to the call to the version-specificserver->ops->parse_lease_buf ?


-- 
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer, filesystems
Red Hat