linux-kernel - Re: [PATCH] proc: move the adding option Ngid to the end of proc/PID/status

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]

Message-ID: <20150421140007.GA22502@p183.telecom.by>
Date:	Tue, 21 Apr 2015 17:00:07 +0300
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	xiaoming.wang@...el.com,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Mel Gorman <mgorman@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] proc: move the adding option Ngid to the end of
 proc/PID/status

On Fri, Apr 17, 2015 at 11:12:59AM -0400, Tejun Heo wrote:
> On Fri, Apr 17, 2015 at 06:05:55PM +0300, Alexey Dobriyan wrote:
> > Moving Ngid to the end of file minimizes risk of breakage.
> 
> Hmmm... how so?

Correctly written parser will be unaffected:

	f = fopen("/proc/self/status", "r");
	while (getline(&buf, &len, f) > 0) {
		if (strncmp(buf, "TracerPid:", 10) != 0)
			continue;
		tracer_pid = ...
	}

Incorrectly written parser

	buf = strchr(buf, '\n');
	buf = strchr(buf, '\n');
	buf = strchr(buf, '\n');
	buf = strchr(buf, '\n');
	buf = strchr(buf, '\n');
	tracer_pid = ...

will be broken.

Moving Ngid to the end will unbreak incorrect parser and all other
incorrect parsers. There are 3 fields before Ngid and 30+ after.
What are the odds?

The only thing moving Ngid to the end is going to break is _another_
incorrect parser expecting Ngid line to be #4.

> The only reason for changing the position is because
> there's this specific breakage.  The goal should be working around
> that specific case while keeping the impact minimum on everyone else.

If there are TWO incorrect parsers, one for TracerPid, another for Ngid,
you CAN'T workaround it. And if you can't workaround you choose code
which was written first, namely, TracerPid one.

In ideal world, Ngid patch will be reverted and it would be up to numa guys
to re-add it so their parsers won't break.

I briefly checked numactl code, it seems to be written correctly
(fopen+readline+strncmp), so there is hope.

> It doesn't matter whether the initial change was good or bad, the
> kernel w/ the new layout is already out in the wild and it has been
> out there for a while.  How is risking changing offsets on most of the
> fields on those kernels a good idea?  Mimize the changes to work
> around the specific case.
> 
> > Correctly written code doesn't care.
> > Code which hardcodes layout won't notice.
> 
> Huh?  Code which hardcodes layout since 1.5 years ago will definitely
> notice.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/