Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1592363 > unrolled thread

Re: [PATCH 0/3] ipc subsystem refcounter conversions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/3] ipc subsystem refcounter conversions Andrew Morton <akpm@linux-foundation.org> - 2017-03-04 01:40 +0100
    RE: [PATCH 0/3] ipc subsystem refcounter conversions "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-06 11:00 +0100

#1592363 — Re: [PATCH 0/3] ipc subsystem refcounter conversions

On Mon, 20 Feb 2017 13:29:46 +0200 Elena Reshetova <elena.reshetova@intel.com> wrote:

> Now when new refcount_t type and API are finally merged
> (see include/linux/refcount.h), the following
> patches convert various refcounters in the ipc susystem from atomic_t
> to refcount_t. By doing this we prevent intentional or accidental
> underflows or overflows that can led to use-after-free vulnerabilities.
> 
> The below patches are fully independent and can be cherry-picked separately.
> Since we convert all kernel subsystems in the same fashion, resulting
> in about 300 patches, we have to group them for sending at least in some
> fashion to be manageable. Please excuse the long cc list.

Again, the refcount_t operations are much more expensive than the bare
atomic_t operations.  I'm reluctant to merge any of these conversions
without either

a) a convincing demonstration that the performance impact is
   sufficiently small (ie: unmeasurable) or

b) a compile-time option to disable the refcount_t operations (make
   them generate the same code as the bare atomic_t ops).  Along with
   some suitably reliable means of preventing people from accidentally
   enabling the debug code in production builds.

[toc] | [next] | [standalone]

#1593137

> On Mon, 20 Feb 2017 13:29:46 +0200 Elena Reshetova
> <elena.reshetova@intel.com> wrote:
> 
> > Now when new refcount_t type and API are finally merged
> > (see include/linux/refcount.h), the following
> > patches convert various refcounters in the ipc susystem from atomic_t
> > to refcount_t. By doing this we prevent intentional or accidental
> > underflows or overflows that can led to use-after-free vulnerabilities.
> >
> > The below patches are fully independent and can be cherry-picked separately.
> > Since we convert all kernel subsystems in the same fashion, resulting
> > in about 300 patches, we have to group them for sending at least in some
> > fashion to be manageable. Please excuse the long cc list.
> 
> Again, the refcount_t operations are much more expensive than the bare
> atomic_t operations.  I'm reluctant to merge any of these conversions
> without either
> 
> a) a convincing demonstration that the performance impact is
>    sufficiently small (ie: unmeasurable) or

Unfortunately pretty much any security options would have a performance impact. 
However, I think it would depend greatly on the actual usage of refcounter, so instead of providing you some generic numbers, I guess we would need to measure it by subsystem. 
Could you please suggest what would be the reasonable test setup/load for testing the ipc impact? 

 
> b) a compile-time option to disable the refcount_t operations (make
>    them generate the same code as the bare atomic_t ops). 

I don't think a compile option to disable refcount operations makes sense, because we already have atomic. 
For places that can't take a performance impact of refcount_t we just have to leave them as atomic_t and don't overcomplicate things IMO.

Along with
>    some suitably reliable means of preventing people from accidentally
>    enabling the debug code in production builds.

Hm... Currently it only does WARN in super rare cases when things don't go well at all (overflow, underflow, increment from zero).
It isn't really debug output anymore in a way that such things need to be detected as early as possible, because they mean bugs. 


Best Regards,
Elena.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web