Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753069Ab2HTSk5 (ORCPT ); Mon, 20 Aug 2012 14:40:57 -0400 Received: from smtpfb2-g21.free.fr ([212.27.42.10]:37195 "EHLO smtpfb2-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751193Ab2HTSkz convert rfc822-to-8bit (ORCPT ); Mon, 20 Aug 2012 14:40:55 -0400 From: Jim Meyering To: Bastien ROUCARIES Cc: linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org, samba-technical@lists.samba.org, Steve French Subject: Re: [PATCH] cifs: remove misleading strncpy: each name has length < 16 In-Reply-To: (Bastien ROUCARIES's message of "Mon, 20 Aug 2012 19:36:26 +0200") References: <1345481724-30108-1-git-send-email-jim@meyering.net> <1345481724-30108-6-git-send-email-jim@meyering.net> Date: Mon, 20 Aug 2012 20:40:10 +0200 Message-ID: <87628dmmph.fsf@rho.meyering.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1954 Lines: 41 Bastien ROUCARIES wrote: > Le 20 ao?t 2012 19:29, "Jim Meyering" a ?crit?: >> >> From: Jim Meyering >> >> Each of the protocols[i].name strings (statically declared above) >> has length less than 16, so this use of strncpy is misleading: >> ? strncpy(pSMB->DialectsArray+count, protocols[i].name, 16); >> Besides, if a new name were added with length N >= 16, the existing >> strncpy-using code would be buggy, creating a ->DialectsArray buffer >> containing N-16+1 unset bytes where the NUL terminator should have >> been. ?Instead, traverse the name only once go get its length, >> use a BUG_ON assertion to enforce the length restriction >> and use memcpy to perform the copy. > > Could you use ARRAY_SIZE instead of hard coding 16? Not that I can see: the DialectsArray member is declared like this: typedef struct negotiate_req { struct smb_hdr hdr; /* wct = 0 */ __le16 ByteCount; unsigned char DialectsArray[1]; } __attribute__((packed)) NEGOTIATE_REQ; ... >> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c >> index 074923c..16a9018 100644 >> --- a/fs/cifs/cifssmb.c >> +++ b/fs/cifs/cifssmb.c >> @@ -441,8 +441,10 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses > *ses) >> >> ? ? ? ? count = 0; >> ? ? ? ? for (i = 0; i < CIFS_NUM_PROT; i++) { >> - ? ? ? ? ? ? ? strncpy(pSMB->DialectsArray+count, protocols[i].name, 16); >> - ? ? ? ? ? ? ? count += strlen(protocols[i].name) + 1; >> + ? ? ? ? ? ? ? size_t len = strlen(protocols[i].name); >> + ? ? ? ? ? ? ? BUG_ON(len >= 16); >> + ? ? ? ? ? ? ? memcpy(pSMB->DialectsArray+count, protocols[i].name, len + 1); >> + ? ? ? ? ? ? ? count += len + 1; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/