Re: Add overflow protection to kref

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Thu, Feb 16, 2012 at 05:06:24PM -0800, Kees Cook wrote:

Any reason you forgot to cc: me on the response?

> On Thu, Feb 16, 2012 at 04:24:05PM -0800, Greg Kroah-Hartman wrote:
> > On Thu, Feb 16, 2012 at 12:45:15PM -0800, Kees Cook wrote:
> > > 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?
> > 
> > a kref is to handle reference counting for an object, so you don't have
> > to constantly "roll your own" all the time using an atomic_t or
> > whatever.  It's the basis for the struct kobject and other object
> > reference counting structures in the kernel for a very long time now.
> > 
> > And in all that time, I've never seen an instance where you can overflow
> > the reference count, so I'm hard pressed to see how changing kref in
> > this manner will help anything at all.
> 
> A quick search gives me:
> CVE-2005-3359: https://bugzilla.redhat.com/show_bug.cgi?id=175769
> CVE-2006-3741: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b8444d00762703e1b6146fce12ce2684885f8bf6

Neither of those are kref issues, just bugs with other types of
counting things.

> And actually an earlier discussion you were actually involved in:
> https://lkml.org/lkml/2008/7/16/300

That wasn't about a kref issue either.  It was also a fun flamefest, but
I don't see how that is relevant here.  What am I missing?

> > So no, I don't recommend changing this logic at all in kref.
> 
> If it's inexpensive and helps defend against problems, it seems sensible to
> add to me.

I have yet to see a patch, so why are we arguing about this?  :)

Again, I don't know of any kref overflows that have ever happened, so
trying to "protect" this type of thing, seems odd to me.

thanks,

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


[Other Archives]     [Linux Kernel Newbies]     [Linux Driver Development]     [Fedora Kernel]     [Linux Kernel Testers]     [Linux SH]     [Linux Omap]     [Linux Kbuild]     [Linux Tape]     [Linux Input]     [Linux Kernel Janitors]     [Linux Kernel Packagers]     [Linux Doc]     [Linux Man Pages]     [Linux API]     [Linux Memory Management]     [Linux Modules]     [Linux Standards]     [Kernel Announce]     [Netdev]     [Git]     [Linux PCI]     Linux CAN Development     [Linux I2C]     [Linux RDMA]     [Linux NUMA]     [Netfilter]     [Netfilter Devel]     [SELinux]     [Bugtraq]     [FIO]     [Linux Perf Users]     [Linux Serial]     [Linux PPP]     [Linux ISDN]     [Linux Next]     [Kernel Stable Commits]     [Linux Tip Commits]     [Kernel MM Commits]     [Linux Security Module]     [Filesystem Development]     [Ext3 Filesystem]     [Linux bcache]     [Ext4 Filesystem]     [Linux BTRFS]     [Linux CEPH Filesystem]     [Linux XFS]     [XFS]     [Linux NFS]     [Linux CIFS]     [Ecryptfs]     [Linux NILFS]     [Linux Cachefs]     [Reiser FS]     [Initramfs]     [Linux FB Devel]     [Linux OpenGL]     [DRI Devel]     [Fastboot]     [Linux RT Users]     [Linux RT Stable]     [eCos]     [Corosync]     [Linux Clusters]     [LVS Devel]     [Hot Plug]     [Linux Virtualization]     [KVM]     [KVM PPC]     [KVM ia64]     [Linux Containers]     [Linux Hexagon]     [Linux Cgroups]     [Util Linux]     [Wireless]     [Linux Bluetooth]     [Bluez Devel]     [Ethernet Bridging]     [Embedded Linux]     [Barebox]     [Linux MMC]     [Linux IIO]     [Sparse]     [Smatch]     [Linux Arch]     [x86 Platform Driver]     [Linux ACPI]     [Linux IBM ACPI]     [LM Sensors]     [CPU Freq]     [Linux Power Management]     [Linmodems]     [Linux DCCP]     [Linux SCTP]     [ALSA Devel]     [Linux USB]     [Linux PA RISC]     [Linux Samsung SOC]     [MIPS Linux]     [IBM S/390 Linux]     [ARM Linux]     [ARM Kernel]     [ARM MSM]     [Tegra Devel]     [Sparc Linux]     [Linux Security]     [Linux Sound]     [Linux Media]     [Video 4 Linux]     [Linux IRDA Users]     [Linux for the blind]     [Linux RAID]     [Linux ATA RAID]     [Device Mapper]     [Linux SCSI]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Linux IDE]     [Linux SMP]     [Linux AXP]     [Linux Alpha]     [Linux M68K]     [Linux ia64]     [Linux 8086]     [Linux x86_64]     [Linux Config]     [Linux Apps]     [Linux MSDOS]     [Linux X.25]     [Linux Crypto]     [DM Crypt]     [Linux Trace Users]     [Linux Btrace]     [Linux Watchdog]     [Utrace Devel]     [Linux C Programming]     [Linux Assembly]     [Dash]     [DWARVES]     [Hail Devel]     [Linux Kernel Debugger]     [Linux gcc]     [Gcc Help]     [X.Org]     [Wine]

Add to Google Powered by Linux

[Older Kernel Discussion]     [Yosemite National Park Forum]     [Large Format Photos]     [Gimp]     [Yosemite Photos]     [Stuff]