linux-ext4 - Re: [PATCH] Fix gcc-wall warnings about strncpy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 15 Aug 2018 23:26:37 -0400
From:   "Theodore Y. Ts'o" <tytso@....edu>
To:     ykp@...tonmail.ch
Cc:     "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] Fix gcc-wall warnings about strncpy

On Wed, Aug 15, 2018 at 01:46:37PM +0000, ykp@...tonmail.ch wrote:
> From 12109693995386c9941129e65078a4305e72936e Mon Sep 17 00:00:00 2001
> From: Vladyslav Tsilytskyi <ykp@...tonmail.ch>
> Date: Wed, 15 Aug 2018 15:25:24 +0200
> Subject: [PATCH] Fix -Wsizeof-pointer-memaccess warnings
> 
> strncpy's last parameter was wrong - there was a length
> of source. In order to use function correctly (and prevent
> buffer overflows) last argument should denote destination's
> buffer capacity.

That's actually not a bug.  It's deliberate.  That's because the
superblock fields are fixed-length char arrays which are null filled,
but if s_volume_name is a 16 byte character array --- and it's legal
for a 16 byte volume label to completely fill s_volume_name,t in which
case s_volume_name is ***NOT*** null filled.

So what we do is make sure buf[] is larger than the superblock field,
zero-fill it, and use strncpy with the 3rd parameter set to size of
the source.  For example:

	if (sb->s_volume_name[0]) {
		memset(buf, 0, sizeof(buf));
		strncpy(buf, sb->s_volume_name, sizeof(sb->s_volume_name));
		strncpy(buf, sb->s_volume_name, sizeof(buf));
	} else
		strcpy(buf, "<none>");

Not all GCC warnings are valid; you can't blindly assuming they are
valid.  That's why they are warnings.  :-)

Cheers,

					- Ted