| Message ID | 1488810076-3754-12-git-send-email-elena.reshetova@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hello. On 03/06/2017 05:20 PM, 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@intel.com> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> [...] > diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h > index 115414c..16c1313 100644 > --- a/drivers/media/pci/cx88/cx88.h > +++ b/drivers/media/pci/cx88/cx88.h > @@ -24,6 +24,7 @@ > #include <linux/i2c-algo-bit.h> > #include <linux/videodev2.h> > #include <linux/kdev_t.h> > +#include <linux/refcount.h> > > #include <media/v4l2-device.h> > #include <media/v4l2-fh.h> > @@ -339,7 +340,7 @@ struct cx8802_dev; > > struct cx88_core { > struct list_head devlist; > - atomic_t refcount; > + refcount_t refcount; Could you please keep the name aligned with above and below? > > /* board name */ > int nr; > MBR, Sergei
> Hello. > > On 03/06/2017 05:20 PM, 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@intel.com> > > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Signed-off-by: David Windsor <dwindsor@gmail.com> > [...] > > diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h > > index 115414c..16c1313 100644 > > --- a/drivers/media/pci/cx88/cx88.h > > +++ b/drivers/media/pci/cx88/cx88.h > > @@ -24,6 +24,7 @@ > > #include <linux/i2c-algo-bit.h> > > #include <linux/videodev2.h> > > #include <linux/kdev_t.h> > > +#include <linux/refcount.h> > > > > #include <media/v4l2-device.h> > > #include <media/v4l2-fh.h> > > @@ -339,7 +340,7 @@ struct cx8802_dev; > > > > struct cx88_core { > > struct list_head devlist; > > - atomic_t refcount; > > + refcount_t refcount; > > Could you please keep the name aligned with above and below? You mean "not aligned" to devlist, but with a shift like it was before? Sure, will fix. Is the patch ok otherwise? Best Regards, Elena. > > > > > /* board name */ > > int nr; > > > > MBR, Sergei
On Mon, Mar 06, 2017 at 04:20:58PM +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@intel.com> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
On 3/7/2017 10:52 AM, Reshetova, Elena 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@intel.com> >>> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> Signed-off-by: David Windsor <dwindsor@gmail.com> >> [...] >>> diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h >>> index 115414c..16c1313 100644 >>> --- a/drivers/media/pci/cx88/cx88.h >>> +++ b/drivers/media/pci/cx88/cx88.h [...] >>> @@ -339,7 +340,7 @@ struct cx8802_dev; >>> >>> struct cx88_core { >>> struct list_head devlist; >>> - atomic_t refcount; >>> + refcount_t refcount; >> >> Could you please keep the name aligned with above and below? > > You mean "not aligned" to devlist, but with a shift like it was before? I mean aligned, like it was before. :-) > Sure, will fix. Is the patch ok otherwise? I haven't noticed anything else... > Best Regards, > Elena. [...] MBR, Sergei
diff --git a/drivers/media/pci/cx88/cx88-cards.c b/drivers/media/pci/cx88/cx88-cards.c index cdfbde2..7fc5f5f 100644 --- a/drivers/media/pci/cx88/cx88-cards.c +++ b/drivers/media/pci/cx88/cx88-cards.c @@ -3670,7 +3670,7 @@ struct cx88_core *cx88_core_create(struct pci_dev *pci, int nr) if (!core) return NULL; - atomic_inc(&core->refcount); + refcount_set(&core->refcount, 1); core->pci_bus = pci->bus->number; core->pci_slot = PCI_SLOT(pci->devfn); core->pci_irqmask = PCI_INT_RISC_RD_BERRINT | PCI_INT_RISC_WR_BERRINT | diff --git a/drivers/media/pci/cx88/cx88-core.c b/drivers/media/pci/cx88/cx88-core.c index 973a9cd4..8bfa5b7 100644 --- a/drivers/media/pci/cx88/cx88-core.c +++ b/drivers/media/pci/cx88/cx88-core.c @@ -1052,7 +1052,7 @@ struct cx88_core *cx88_core_get(struct pci_dev *pci) mutex_unlock(&devlist); return NULL; } - atomic_inc(&core->refcount); + refcount_inc(&core->refcount); mutex_unlock(&devlist); return core; } @@ -1073,7 +1073,7 @@ void cx88_core_put(struct cx88_core *core, struct pci_dev *pci) release_mem_region(pci_resource_start(pci, 0), pci_resource_len(pci, 0)); - if (!atomic_dec_and_test(&core->refcount)) + if (!refcount_dec_and_test(&core->refcount)) return; mutex_lock(&devlist); diff --git a/drivers/media/pci/cx88/cx88.h b/drivers/media/pci/cx88/cx88.h index 115414c..16c1313 100644 --- a/drivers/media/pci/cx88/cx88.h +++ b/drivers/media/pci/cx88/cx88.h @@ -24,6 +24,7 @@ #include <linux/i2c-algo-bit.h> #include <linux/videodev2.h> #include <linux/kdev_t.h> +#include <linux/refcount.h> #include <media/v4l2-device.h> #include <media/v4l2-fh.h> @@ -339,7 +340,7 @@ struct cx8802_dev; struct cx88_core { struct list_head devlist; - atomic_t refcount; + refcount_t refcount; /* board name */ int nr;