linux-kernel - Re: [PATCH][RFC] Handle improbable possibility of io

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20090429081558.acaa2a9a.akpm@linux-foundation.org>
Date:	Wed, 29 Apr 2009 08:15:58 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Nikanth Karthikesan <knikanth@...ell.com>
Cc:	Jens Axboe <jens.axboe@...cle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH][RFC] Handle improbable possibility of
 io_context->refcount overflow

On Wed, 29 Apr 2009 15:33:06 +0530 Nikanth Karthikesan <knikanth@...ell.com> wrote:

> On Wednesday 29 April 2009 13:29:30 Andrew Morton wrote:
> > On Wed, 29 Apr 2009 12:21:39 +0530 Nikanth Karthikesan <knikanth@...ell.com> wrote:
> > > Hi Jens
> > >
> > > Currently io_context has an atomic_t(int) as refcount. In case of cfq,
> > > for each device a task does I/O, a reference to the io_context would be
> > > taken. And when there are multiple process sharing io_contexts(CLONE_IO)
> > > would also have a reference to the same io_context. Theoretically the
> > > possible maximum number of processes sharing the same io_context + the
> > > number of disks/cfq_data referring to the same io_context can overflow
> > > the 32-bit counter on a very high-end machine. Even though it is an
> > > improbable case, let us make it difficult by changing the refcount to
> > > atomic64_t(long).
> >
> > Sorry, atomic64_t isn't implemented on 32 bit architectures.
> >
> > Perhaps it should be, but I expect it'd be pretty slow.
> 
> Oh! Sorry, I didn't notice the #ifdef earlier. I guess thats why there is only
> a single in-tree user for atomic64_t!

Yes, it's a bit irritating.

> In this case, could we make it atomic64_t only on 64-bit architectures and
> keep it as atomic_t on 32-bit machines?

Sure.

> Something like the attached patch.

Check out atomic_long_t ;)

> I wonder whether we should also add BUG_ON's whenever the refcount is about to
> wrap? Or try to handle it gracefully. Another approach would be to impose an
> artificial limit on the no of tasks that could share an io_context. Or resort
> to lock protection. The problem is not very serious/common.
> 

For a long time there was a debug patch in -mm which would warn if
atomic_dec() ever took any atomic_t from zero to -1.  I don't think it
ever triggered false positives and it did find a couple of bugs.

I forget what happened to the patch - probably it died when the atomic
code got altered.

It could well be that a similar kernel-wide check for atomic_inc()
overflows would be similarly useful.

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