Date: Thu, 16 Feb 2012 12:45:15 -0800
From: Kees Cook <keescook@chromium.org>
To: Ubuntu security discussion <ubuntu-hardened@lists.ubuntu.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        kernel-hardening@lists.openwall.com, linux-kernel@vger.kernel.org
Subject: Re: Add overflow protection to kref
Message-ID: <20120216204515.GH20420@outflux.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAEXv5_gkTsPHDFh+wQqD3P3D-Z+uCN-_1bVyHfHV=u7bS-tgeA@mail.gmail.com>
Organization: ChromeOS
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2860
Lines: 86

Hi,

[This should probably be discussed on LKML for an even wider audience, so
I've added a CC for it there.]

On Thu, Feb 16, 2012 at 09:02:13AM -0500, David Windsor wrote:
> Hi,
> 
> We are attempting to add various grsecurity/PAX features to upstream
> Ubuntu kernels.

This didn't parse quite right for me. I think you meant that the intent
is to get these features into the upstream Linux kernel, with potential
staging in Ubuntu kernels.

(Also s/PAX/PaX/g)

> The PAX folks added refcount overflow protection by inserting
> architecture-specific code in the increment paths of atomic_t.  For
> instance:
> 
> static inline void atomic_inc(atomic_t *v)
>  {
> 	asm volatile(LOCK_PREFIX "incl %0\n"
> 
> #ifdef CONFIG_PAX_REFCOUNT
> 		     "jno 0f\n"
> 		     LOCK_PREFIX "decl %0\n"
> 		     "int $4\n0:\n"
> 		     _ASM_EXTABLE(0b, 0b)
> #endif
> 
> 		     : "+m" (v->counter));
> }
> 
> There are two distinct classes of users we need to consider here:
> those who use atomic_t for reference counters and those who use
> atomic_t for keeping track of statistics, like performance counters,
> etc.; it makes little sense to overflow a performance counter, so we
> shouldn't subject those users to the same protections as imposed on
> actual reference counters.  The solution implemented by PAX is to
> create a family of *_unchecked() functions and to patch
> statistics-based users of atomic_t to use this interface.
> 
> PAX refcount overflow protection was developed before kref was
> created.  I'd like to move overflow protection out of atomic_t and
> into kref and gradually migrate atomic_t users to kref, leaving
> atomic_t for those users who don't need overflow protection (e.g.
> statistics-based counters).

For people new to this, can you give an overview of what attacks are foiled
by adding overflow protection?

> I realize that there are many users of atomic_t needing overflow
> protection, but the move to kref seems like the right thing to do in
> this case.
> 
> Leaving the semantics of overflow detection aside for the moment, what
> are everyone's thoughts on adding overflow protection to kref rather
> than to atomic_t?

Why was kref introduced? Or rather, how is kref currently different from
atomic_t?

> Also, I cherrypicked the refcount protection feature directly from the
> PAX patch, with the original atomic_t protections in place, before
> considering kref.  If anyone is interested, I can post that patch.
> 
> Thanks,
> David Windsor
> 
> -- 
> PGP: 6141 5FFD 11AE 9844 153E ?F268 7C98 7268 6B19 6CC9

Thanks for bringing this up!

-Kees

-- 
Kees Cook
ChromeOS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/