All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 06/10] misc: fix strncpy length complaints
Date: Mon, 22 Apr 2019 16:04:59 -0500	[thread overview]
Message-ID: <0017a715-9a0e-f606-0e88-6226db06920d@sandeen.net> (raw)
In-Reply-To: <20190422205703.GC4676@magnolia>

On 4/22/19 3:57 PM, Darrick J. Wong wrote:
> On Mon, Apr 22, 2019 at 03:48:03PM -0500, Eric Sandeen wrote:
>> On 4/22/19 10:45 AM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Fix a number of complaints about feeding sizeof(dest) directly to
>>> strncpy.  We do this by declaring the char arrays to be one larger
>>> than necessary and subtracting one, to ensure that we never overfill
>>> the buffer.
>>>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> Hm, you don't actually do what the changelog says, do you?  No buffer
>> changes size in this patch.
> 
> I'll change it to:
> 
> "Fix a number of complaints about feeding sizeof(dest) directly to
> strncpy.  We do this by feeding strncpy the length of the buffer minus
> one, having checked that the allocated space are long enough."

sounds good, I'll just fix that here.

>> And FWIW, neither of these was actually a real problem (fgets guarantees
>> a null in the string) but hey, whatever makes gcc happy I guess?
>>
>> (I think we lose a char in the quota array, but 511 chars ought to be
>> enough for anybody right?)
> 
> Well... the quota one is reading lines straight out of a file, right?
> So that probably ought to be char buf[PATH_MAX + length of other junk in
> the line], though with the current lower limit now nobody's complained
> so maybe it's fine....
> 
> --D
> 
>> Patch seems fine, though.
>>
>>> ---
>>>  mkfs/xfs_mkfs.c |   13 +++++++++++--
>>>  quota/edit.c    |    9 ++++++---
>>>  2 files changed, 17 insertions(+), 5 deletions(-)
>>>
>>>
>>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>>> index 3e2ef92d..db3ad38e 100644
>>> --- a/mkfs/xfs_mkfs.c
>>> +++ b/mkfs/xfs_mkfs.c
>>> @@ -3270,8 +3270,17 @@ finish_superblock_setup(
>>>  	struct xfs_mount	*mp,
>>>  	struct xfs_sb		*sbp)
>>>  {
>>> -	if (cfg->label)
>>> -		strncpy(sbp->sb_fname, cfg->label, sizeof(sbp->sb_fname));
>>> +	if (cfg->label) {
>>> +		size_t		label_len;
>>> +
>>> +		/*
>>> +		 * Labels are null terminated unless the string fits exactly
>>> +		 * in the label field, so assume sb_fname is zeroed and then
>>> +		 * do a memcpy because the destination isn't a normal C string.
>>> +		 */
>>> +		label_len = min(sizeof(sbp->sb_fname), strlen(cfg->label));
>>> +		memcpy(sbp->sb_fname, cfg->label, label_len);
>>> +	}
>>>  
>>>  	sbp->sb_dblocks = cfg->dblocks;
>>>  	sbp->sb_rblocks = cfg->rtblocks;
>>> diff --git a/quota/edit.c b/quota/edit.c
>>> index b10a5b34..f9938b8a 100644
>>> --- a/quota/edit.c
>>> +++ b/quota/edit.c
>>> @@ -368,8 +368,7 @@ restore_file(
>>>  	uint		type)
>>>  {
>>>  	char		buffer[512];
>>> -	char		devbuffer[512];
>>> -	char		*dev = NULL;
>>> +	char		dev[512];
>>>  	uint		mask;
>>>  	int		cnt;
>>>  	uint32_t	id;
>>> @@ -377,7 +376,11 @@ restore_file(
>>>  
>>>  	while (fgets(buffer, sizeof(buffer), fp) != NULL) {
>>>  		if (strncmp("fs = ", buffer, 5) == 0) {
>>> -			dev = strncpy(devbuffer, buffer+5, sizeof(devbuffer));
>>> +			/*
>>> +			 * Copy the device name to dev, strip off the trailing
>>> +			 * newline, and move on to the next line.
>>> +			 */
>>> +			strncpy(dev, buffer + 5, sizeof(dev) - 1);
>>>  			dev[strlen(dev) - 1] = '\0';
>>>  			continue;
>>>  		}
>>>
> 

  reply	other threads:[~2019-04-22 21:05 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-22 15:44 [PATCH v3 00/10] xfsprogs-5.0: fix various problems Darrick J. Wong
2019-04-22 15:44 ` [PATCH 01/10] scrub: fix Makefile targets which depend on builddefs Darrick J. Wong
2019-04-22 18:27   ` Eric Sandeen
2019-04-22 18:28   ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 02/10] xfs_info: use findmnt to handle mounted block devices Darrick J. Wong
2019-04-22 18:35   ` Eric Sandeen
2019-04-22 19:27   ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 03/10] xfs_repair: correctly account for free space btree shrinks when fixing freelist Darrick J. Wong
2019-04-22 19:24   ` Eric Sandeen
2019-04-22 19:36   ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 04/10] libxfs: retain ifork_ops when flushing inode Darrick J. Wong
2019-04-22 19:40   ` Bill O'Donnell
2019-04-22 19:45   ` Eric Sandeen
2019-10-02  6:00   ` Arkadiusz Miśkiewicz
2019-04-22 15:45 ` [PATCH 05/10] libxfs: drop the ifork_ops parameter from _inode_verify_forks Darrick J. Wong
2019-04-22 19:43   ` Bill O'Donnell
2019-04-22 20:49   ` Eric Sandeen
2019-04-22 15:45 ` [PATCH 06/10] misc: fix strncpy length complaints Darrick J. Wong
2019-04-22 20:48   ` Eric Sandeen
2019-04-22 20:57     ` Darrick J. Wong
2019-04-22 21:04       ` Eric Sandeen [this message]
2019-04-22 21:07   ` Eric Sandeen
2019-04-23 15:07   ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 07/10] libxfs: refactor buffer item release code Darrick J. Wong
2019-04-22 21:26   ` Eric Sandeen
2019-04-22 21:35     ` Darrick J. Wong
2019-04-22 21:40       ` Eric Sandeen
2019-04-23 20:51   ` [PATCH v2 " Darrick J. Wong
2019-04-23 20:56     ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 08/10] libxfs: don't touch buffer log item pointer when flushing inode log item Darrick J. Wong
2019-04-23 17:56   ` Eric Sandeen
2019-04-23 20:52   ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 09/10] libxfs: fix buffer log item lifetime weirdness Darrick J. Wong
2019-04-23 21:15   ` Bill O'Donnell
2019-04-22 15:45 ` [PATCH 10/10] libxfs: shorten inode item lifetime Darrick J. Wong
2019-04-23 21:22   ` Bill O'Donnell
2019-04-23 21:04 ` [PATCH 11/10] libfrog: fix memory leak in bitmap_free Darrick J. Wong
2019-04-23 21:23   ` Bill O'Donnell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0017a715-9a0e-f606-0e88-6226db06920d@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.