Re: [PATCH] cifssmb: use struct_offset in memcpy calculations
Rosen Penev <[email protected]> Thu, 30 Apr 2026 17:27:14 -0700
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.cifs,gmane.network.samba.internals |
|---|---|
| Message-ID | <CAKxU2N_m8+Rd8Q0Ty7sk0_bANdyagy9KTc8GM=hHu=FbwRbaYg@mail.gmail.com> |
On Thu, Apr 30, 2026 at 5:09 PM Steve French <[email protected]> wrote: > > Seems clearer to leave in the cast so it is a memcpy from char * to char * Will fix in v2. Can't say I agree though. > > memcpy((char *) pFindData, > > On Thu, Apr 30, 2026 at 5:06 PM Rosen Penev <[email protected]> wrote: > > > > It seems W=1/FORTIFY_SOURCE is getting confused about the math here. > > > > Use struct_offset to point FORTIFY_SOURCE to the correct place. > > > > Remove response_data variables and directly apply to memcpy. Simpler > > and avoids excess casting. > > > > Signed-off-by: Rosen Penev <[email protected]> > > --- > > fs/smb/client/cifssmb.c | 40 +++++++++++++--------------------------- > > 1 file changed, 13 insertions(+), 27 deletions(-) > > > > diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c > > index 3990a9012264..8598cec5ebbd 100644 > > --- a/fs/smb/client/cifssmb.c > > +++ b/fs/smb/client/cifssmb.c > > @@ -4090,9 +4090,9 @@ CIFSSMBQFileInfo(const unsigned int xid, struct cifs_tcon *tcon, > > get_bcc(&pSMBr->hdr), 40); > > else if (pFindData) { > > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > > - memcpy((char *) pFindData, > > - (char *) &pSMBr->hdr.Protocol + > > - data_offset, sizeof(FILE_ALL_INFO)); > > + memcpy(pFindData, > > + (char *)pSMBr + struct_offset(pSMBr, hdr.Protocol) + data_offset, > > + sizeof(FILE_ALL_INFO)); > > } else > > rc = -ENOMEM; > > } > > @@ -4264,9 +4264,8 @@ CIFSSMBUnixQFileInfo(const unsigned int xid, struct cifs_tcon *tcon, > > get_bcc(&pSMBr->hdr), sizeof(FILE_UNIX_BASIC_INFO)); > > } else { > > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > > - memcpy((char *) pFindData, > > - (char *) &pSMBr->hdr.Protocol + > > - data_offset, > > + memcpy(pFindData, > > + (char *)pSMBr + struct_offset(pSMBr, hdr.Protocol) + data_offset, > > sizeof(FILE_UNIX_BASIC_INFO)); > > } > > } > > @@ -4349,9 +4348,8 @@ CIFSSMBUnixQPathInfo(const unsigned int xid, struct cifs_tcon *tcon, > > get_bcc(&pSMBr->hdr), sizeof(FILE_UNIX_BASIC_INFO)); > > } else { > > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > > - memcpy((char *) pFindData, > > - (char *) &pSMBr->hdr.Protocol + > > - data_offset, > > + memcpy(pFindData, > > + (char *)pSMBr + struct_offset(pSMBr, hdr.Protocol) + data_offset, > > sizeof(FILE_UNIX_BASIC_INFO)); > > } > > } > > @@ -5079,7 +5077,6 @@ CIFSSMBQFSAttributeInfo(const unsigned int xid, struct cifs_tcon *tcon) > > /* level 0x105 SMB_QUERY_FILE_SYSTEM_INFO */ > > TRANSACTION2_QFSI_REQ *pSMB = NULL; > > TRANSACTION2_QFSI_RSP *pSMBr = NULL; > > - FILE_SYSTEM_ATTRIBUTE_INFO *response_data; > > unsigned int in_len; > > int rc = 0; > > int bytes_returned = 0; > > @@ -5130,11 +5127,8 @@ CIFSSMBQFSAttributeInfo(const unsigned int xid, struct cifs_tcon *tcon) > > get_bcc(&pSMBr->hdr), 13); > > } else { > > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > > - response_data = > > - (FILE_SYSTEM_ATTRIBUTE_INFO > > - *) (((char *) &pSMBr->hdr.Protocol) + > > - data_offset); > > - memcpy(&tcon->fsAttrInfo, response_data, > > + memcpy(&tcon->fsAttrInfo, > > + (char *)pSMBr + struct_offset(pSMBr, hdr.Protocol) + data_offset, > > sizeof(FILE_SYSTEM_ATTRIBUTE_INFO)); > > } > > } > > @@ -5152,7 +5146,6 @@ CIFSSMBQFSDeviceInfo(const unsigned int xid, struct cifs_tcon *tcon) > > /* level 0x104 SMB_QUERY_FILE_SYSTEM_INFO */ > > TRANSACTION2_QFSI_REQ *pSMB = NULL; > > TRANSACTION2_QFSI_RSP *pSMBr = NULL; > > - FILE_SYSTEM_DEVICE_INFO *response_data; > > unsigned int in_len; > > int rc = 0; > > int bytes_returned = 0; > > @@ -5205,11 +5198,8 @@ CIFSSMBQFSDeviceInfo(const unsigned int xid, struct cifs_tcon *tcon) > > sizeof(FILE_SYSTEM_DEVICE_INFO)); > > else { > > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > > - response_data = > > - (FILE_SYSTEM_DEVICE_INFO *) > > - (((char *) &pSMBr->hdr.Protocol) + > > - data_offset); > > - memcpy(&tcon->fsDevInfo, response_data, > > + memcpy(&tcon->fsDevInfo, > > + (char *)pSMBr + struct_offset(pSMBr, hdr.Protocol) + data_offset, > > sizeof(FILE_SYSTEM_DEVICE_INFO)); > > } > > } > > @@ -5227,7 +5217,6 @@ CIFSSMBQFSUnixInfo(const unsigned int xid, struct cifs_tcon *tcon) > > /* level 0x200 SMB_QUERY_CIFS_UNIX_INFO */ > > TRANSACTION2_QFSI_REQ *pSMB = NULL; > > TRANSACTION2_QFSI_RSP *pSMBr = NULL; > > - FILE_SYSTEM_UNIX_INFO *response_data; > > unsigned int in_len; > > int rc = 0; > > int bytes_returned = 0; > > @@ -5277,11 +5266,8 @@ CIFSSMBQFSUnixInfo(const unsigned int xid, struct cifs_tcon *tcon) > > get_bcc(&pSMBr->hdr), 13); > > } else { > > __u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset); > > - response_data = > > - (FILE_SYSTEM_UNIX_INFO > > - *) (((char *) &pSMBr->hdr.Protocol) + > > - data_offset); > > - memcpy(&tcon->fsUnixInfo, response_data, > > + memcpy(&tcon->fsUnixInfo, > > + (char *)pSMBr + struct_offset(pSMBr, hdr.Protocol) + data_offset, > > sizeof(FILE_SYSTEM_UNIX_INFO)); > > } > > } > > -- > > 2.54.0 > > > > > > > -- > Thanks, > > Steve