linux-kernel - RE: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount

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

Message-ID: <2236FBA76BA1254E88B949DDB74E612B41C558E6@IRSMSX102.ger.corp.intel.com>
Date:   Tue, 7 Mar 2017 14:48:43 +0000
From:   "Reshetova, Elena" <elena.reshetova@...el.com>
To:     Sakari Ailus <sakari.ailus@....fi>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux1394-devel@...ts.sourceforge.net" 
        <linux1394-devel@...ts.sourceforge.net>,
        "linux-bcache@...r.kernel.org" <linux-bcache@...r.kernel.org>,
        "linux-raid@...r.kernel.org" <linux-raid@...r.kernel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
        "fcoe-devel@...n-fcoe.org" <fcoe-devel@...n-fcoe.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "open-iscsi@...glegroups.com" <open-iscsi@...glegroups.com>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "target-devel@...r.kernel.org" <target-devel@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        Hans Liljestrand <ishkamiel@...il.com>,
        Kees Cook <keescook@...omium.org>,
        David Windsor <dwindsor@...il.com>
Subject: RE: [PATCH 13/29] drivers, media: convert
 vb2_vmarea_handler.refcount from atomic_t to refcount_t

> Hi Elena,
> 
> On Mon, Mar 06, 2017 at 04:21:00PM +0200, Elena Reshetova wrote:
> > refcount_t type and corresponding API should be
> > used instead of atomic_t when the variable is used as
> > a reference counter. This allows to avoid accidental
> > refcounter overflows that might lead to use-after-free
> > situations.
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@...el.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@...il.com>
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> > Signed-off-by: David Windsor <dwindsor@...il.com>
> > ---
> >  drivers/media/v4l2-core/videobuf2-memops.c | 6 +++---
> >  include/media/videobuf2-memops.h           | 3 ++-
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf2-memops.c b/drivers/media/v4l2-
> core/videobuf2-memops.c
> > index 1cd322e..4bb8424 100644
> > --- a/drivers/media/v4l2-core/videobuf2-memops.c
> > +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> > @@ -96,10 +96,10 @@ static void vb2_common_vm_open(struct
> vm_area_struct *vma)
> >  	struct vb2_vmarea_handler *h = vma->vm_private_data;
> >
> >  	pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> > -	       __func__, h, atomic_read(h->refcount), vma->vm_start,
> > +	       __func__, h, refcount_read(h->refcount), vma->vm_start,
> >  	       vma->vm_end);
> >
> > -	atomic_inc(h->refcount);
> > +	refcount_inc(h->refcount);
> >  }
> >
> >  /**
> > @@ -114,7 +114,7 @@ static void vb2_common_vm_close(struct
> vm_area_struct *vma)
> >  	struct vb2_vmarea_handler *h = vma->vm_private_data;
> >
> >  	pr_debug("%s: %p, refcount: %d, vma: %08lx-%08lx\n",
> > -	       __func__, h, atomic_read(h->refcount), vma->vm_start,
> > +	       __func__, h, refcount_read(h->refcount), vma->vm_start,
> >  	       vma->vm_end);
> >
> >  	h->put(h->arg);
> > diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-
> memops.h
> > index 36565c7a..a6ed091 100644
> > --- a/include/media/videobuf2-memops.h
> > +++ b/include/media/videobuf2-memops.h
> > @@ -16,6 +16,7 @@
> >
> >  #include <media/videobuf2-v4l2.h>
> >  #include <linux/mm.h>
> > +#include <linux/refcount.h>
> >
> >  /**
> >   * struct vb2_vmarea_handler - common vma refcount tracking handler
> > @@ -25,7 +26,7 @@
> >   * @arg:	argument for @put callback
> >   */
> >  struct vb2_vmarea_handler {
> > -	atomic_t		*refcount;
> > +	refcount_t		*refcount;
> 
> This is a pointer to refcount, not refcount itself. The refcount is part of
> a memory type specific struct, the types that you change in the following
> three patches. I guess it would still compile and work as separate patches
> but you'd sure get warnings at least.

Actually it doesn't compile without this patch if I remember it correctly back in past when I was initially splitting the patches per variable. 

> 
> How about merging this and the three following patches that change the memop
> refcount types?

Sounds good!

Best Regards,
Elena.
> 
> >  	void			(*put)(void *arg);
> >  	void			*arg;
> >  };
> 
> --
> Kind regards,
> 
> Sakari Ailus
> e-mail: sakari.ailus@....fi	XMPP: sailus@...iisi.org.uk