linux-kernel - Re: [PATCH] drm: make variable named "refcount" atomic, like most refcounts in the kernel.

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] [day] [month] [year] [list]

Message-ID: <20140426234204.GD17562@mguzik.redhat.com>
Date:	Sun, 27 Apr 2014 01:42:05 +0200
From:	Mateusz Guzik <mguzik@...hat.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Lionel Debroux <lionel_debroux@...oo.fr>,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH] drm: make variable named "refcount" atomic, like most
 refcounts in the kernel.

On Sun, Apr 27, 2014 at 12:00:56AM +0100, Al Viro wrote:
> (..)Non-atomic variant would be
> 	if (++*p < 0) {
> 		--*p;
> 		whine
> 		send SIGKILL to ourselves
> 	}
> which is nowhere near a sane mitigation in this case.  Much saner one would
> be (*IF* the overflow is, indeed, possible and if simple s/int/long/ wouldn't
> be a better fix) something along the lines of
> 	mutex_lock(&item->mutex);
> 	if (unlikely(item->refcount == INT_MAX)) {
> 		ret = -EINVAL;
> 		<maybe whine and/or raise SIGKILL>
> 		goto out_err;
> 	}
> 	<what we do there right now>
> 
> Mind you, I would be quite surprised if it turned out to be even
> theoretically possible to get that overflow here (judging by the look
> of callers, you'll run out of device numbers long before), but that's
> a separate story.
> 
> The point is, your "useful feature" is anything but, when applied without
> careful analysis of the situation; it's *not* a universal improvement.

I would argue even functions doing mere ptr->count++ and so on would be
useful.

For instance people who are fuzzing kernels looking for refcount
leaks/overupts could place low thresholds in these functions (along with
atomic ops) to increase chances that problems will manifest themselves.

(and this would help more people who report such problems)

The kernel locking itself up afterwards is not a problem for them.

That is provided there is enough hand-coded refcount code, if this would
be the only consumer (which will most likely never leak anyway) then this
is defnitely not worth it.

-- 
Mateusz Guzik
--
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/