| Message ID | YzzjKyHDuFoQAVCu@work |
|---|---|
| State | New |
| Headers | show |
| Series | [next] cifs: Replace a couple of one-element arrays with flexible-array members | expand |
On Tue, Oct 04, 2022 at 08:51:39PM -0500, Gustavo A. R. Silva wrote: > One-element arrays are deprecated, and we are replacing them with flexible > array members instead. So, replace one-element arrays with flexible-array > member in structs negotiate_req and extended_response, and refactor the > rest of the code, accordingly. > > Also, make use of the DECLARE_FLEX_ARRAY() helper to declare flexible > array member EncryptionKey in union u. This new helper allows for > flexible-array members in unions. > > Change pointer notation to proper array notation in a call to memcpy() > where flexible-array member DialectsArray is being used as destination > argument. > > Important to mention is that doing a build before/after this patch results > in no binary output differences. > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE > routines on memcpy() and help us make progress towards globally > enabling -fstrict-flex-arrays=3 [1]. > > Link: https://github.com/KSPP/linux/issues/79 > Link: https://github.com/KSPP/linux/issues/229 > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Looks good to me; thanks! Reviewed-by: Kees Cook <keescook@chromium.org>
On Wed, 5 Oct 2022 at 12:31, Gustavo A. R. Silva <gustavoars@kernel.org> wrote: > > One-element arrays are deprecated, and we are replacing them with flexible > array members instead. So, replace one-element arrays with flexible-array > member in structs negotiate_req and extended_response, and refactor the > rest of the code, accordingly. > > Also, make use of the DECLARE_FLEX_ARRAY() helper to declare flexible > array member EncryptionKey in union u. This new helper allows for > flexible-array members in unions. > > Change pointer notation to proper array notation in a call to memcpy() > where flexible-array member DialectsArray is being used as destination > argument. > > Important to mention is that doing a build before/after this patch results > in no binary output differences. Looks good to me. Reviewed-by me Thanks for verifying that it does not change the binary utput. > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE > routines on memcpy() and help us make progress towards globally > enabling -fstrict-flex-arrays=3 [1]. > > Link: https://github.com/KSPP/linux/issues/79 > Link: https://github.com/KSPP/linux/issues/229 > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > fs/cifs/cifspdu.h | 7 ++++--- > fs/cifs/cifssmb.c | 2 +- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h > index aeba371c4c70..d1abaeea974a 100644 > --- a/fs/cifs/cifspdu.h > +++ b/fs/cifs/cifspdu.h > @@ -483,7 +483,7 @@ put_bcc(__u16 count, struct smb_hdr *hdr) > typedef struct negotiate_req { > struct smb_hdr hdr; /* wct = 0 */ > __le16 ByteCount; > - unsigned char DialectsArray[1]; > + unsigned char DialectsArray[]; > } __attribute__((packed)) NEGOTIATE_REQ; > > #define MIN_TZ_ADJ (15 * 60) /* minimum grid for timezones in seconds */ > @@ -508,13 +508,14 @@ typedef struct negotiate_rsp { > __u8 EncryptionKeyLength; > __u16 ByteCount; > union { > - unsigned char EncryptionKey[1]; /* cap extended security off */ > + /* cap extended security off */ > + DECLARE_FLEX_ARRAY(unsigned char, EncryptionKey); > /* followed by Domain name - if extended security is off */ > /* followed by 16 bytes of server GUID */ > /* then security blob if cap_extended_security negotiated */ > struct { > unsigned char GUID[SMB1_CLIENT_GUID_SIZE]; > - unsigned char SecurityBlob[1]; > + unsigned char SecurityBlob[]; > } __attribute__((packed)) extended_response; > } __attribute__((packed)) u; > } __attribute__((packed)) NEGOTIATE_RSP; > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 7aa91e272027..7a808e41b1b8 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -465,7 +465,7 @@ CIFSSMBNegotiate(const unsigned int xid, > for (i = 0; i < CIFS_NUM_PROT; i++) { > size_t len = strlen(protocols[i].name) + 1; > > - memcpy(pSMB->DialectsArray+count, protocols[i].name, len); > + memcpy(&pSMB->DialectsArray[count], protocols[i].name, len); > count += len; > } > inc_rfc1001_len(pSMB, count); > -- > 2.34.1 >
merged into cifs-2.6.git for-next On Tue, Oct 4, 2022 at 10:17 PM Kees Cook <keescook@chromium.org> wrote: > > On Tue, Oct 04, 2022 at 08:51:39PM -0500, Gustavo A. R. Silva wrote: > > One-element arrays are deprecated, and we are replacing them with flexible > > array members instead. So, replace one-element arrays with flexible-array > > member in structs negotiate_req and extended_response, and refactor the > > rest of the code, accordingly. > > > > Also, make use of the DECLARE_FLEX_ARRAY() helper to declare flexible > > array member EncryptionKey in union u. This new helper allows for > > flexible-array members in unions. > > > > Change pointer notation to proper array notation in a call to memcpy() > > where flexible-array member DialectsArray is being used as destination > > argument. > > > > Important to mention is that doing a build before/after this patch results > > in no binary output differences. > > > > This helps with the ongoing efforts to tighten the FORTIFY_SOURCE > > routines on memcpy() and help us make progress towards globally > > enabling -fstrict-flex-arrays=3 [1]. > > > > Link: https://github.com/KSPP/linux/issues/79 > > Link: https://github.com/KSPP/linux/issues/229 > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > > Looks good to me; thanks! > > Reviewed-by: Kees Cook <keescook@chromium.org> > > -- > Kees Cook
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h index aeba371c4c70..d1abaeea974a 100644 --- a/fs/cifs/cifspdu.h +++ b/fs/cifs/cifspdu.h @@ -483,7 +483,7 @@ put_bcc(__u16 count, struct smb_hdr *hdr) typedef struct negotiate_req { struct smb_hdr hdr; /* wct = 0 */ __le16 ByteCount; - unsigned char DialectsArray[1]; + unsigned char DialectsArray[]; } __attribute__((packed)) NEGOTIATE_REQ; #define MIN_TZ_ADJ (15 * 60) /* minimum grid for timezones in seconds */ @@ -508,13 +508,14 @@ typedef struct negotiate_rsp { __u8 EncryptionKeyLength; __u16 ByteCount; union { - unsigned char EncryptionKey[1]; /* cap extended security off */ + /* cap extended security off */ + DECLARE_FLEX_ARRAY(unsigned char, EncryptionKey); /* followed by Domain name - if extended security is off */ /* followed by 16 bytes of server GUID */ /* then security blob if cap_extended_security negotiated */ struct { unsigned char GUID[SMB1_CLIENT_GUID_SIZE]; - unsigned char SecurityBlob[1]; + unsigned char SecurityBlob[]; } __attribute__((packed)) extended_response; } __attribute__((packed)) u; } __attribute__((packed)) NEGOTIATE_RSP; diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 7aa91e272027..7a808e41b1b8 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -465,7 +465,7 @@ CIFSSMBNegotiate(const unsigned int xid, for (i = 0; i < CIFS_NUM_PROT; i++) { size_t len = strlen(protocols[i].name) + 1; - memcpy(pSMB->DialectsArray+count, protocols[i].name, len); + memcpy(&pSMB->DialectsArray[count], protocols[i].name, len); count += len; } inc_rfc1001_len(pSMB, count);
One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element arrays with flexible-array member in structs negotiate_req and extended_response, and refactor the rest of the code, accordingly. Also, make use of the DECLARE_FLEX_ARRAY() helper to declare flexible array member EncryptionKey in union u. This new helper allows for flexible-array members in unions. Change pointer notation to proper array notation in a call to memcpy() where flexible-array member DialectsArray is being used as destination argument. Important to mention is that doing a build before/after this patch results in no binary output differences. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/229 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- fs/cifs/cifspdu.h | 7 ++++--- fs/cifs/cifssmb.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-)