diff mbox

[11/29] drivers, media: convert cx88_core.refcount from atomic_t to refcount_t

Message ID 1488810076-3754-12-git-send-email-elena.reshetova@intel.com (mailing list archive)
State Superseded
Headers show

Commit Message

Reshetova, Elena March 6, 2017, 2:20 p.m. UTC
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>
---
 drivers/media/pci/cx88/cx88-cards.c | 2 +-
 drivers/media/pci/cx88/cx88-core.c  | 4 ++--
 drivers/media/pci/cx88/cx88.h       | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

Comments

Sergei Shtylyov March 6, 2017, 4:26 p.m. UTC | #1
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

--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reshetova, Elena March 7, 2017, 7:52 a.m. UTC | #2
PiBIZWxsby4NCj4gDQo+IE9uIDAzLzA2LzIwMTcgMDU6MjAgUE0sIEVsZW5hIFJlc2hldG92YSB3
cm90ZToNCj4gDQo+ID4gcmVmY291bnRfdCB0eXBlIGFuZCBjb3JyZXNwb25kaW5nIEFQSSBzaG91
bGQgYmUNCj4gPiB1c2VkIGluc3RlYWQgb2YgYXRvbWljX3Qgd2hlbiB0aGUgdmFyaWFibGUgaXMg
dXNlZCBhcw0KPiA+IGEgcmVmZXJlbmNlIGNvdW50ZXIuIFRoaXMgYWxsb3dzIHRvIGF2b2lkIGFj
Y2lkZW50YWwNCj4gPiByZWZjb3VudGVyIG92ZXJmbG93cyB0aGF0IG1pZ2h0IGxlYWQgdG8gdXNl
LWFmdGVyLWZyZWUNCj4gPiBzaXR1YXRpb25zLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogRWxl
bmEgUmVzaGV0b3ZhIDxlbGVuYS5yZXNoZXRvdmFAaW50ZWwuY29tPg0KPiA+IFNpZ25lZC1vZmYt
Ynk6IEhhbnMgTGlsamVzdHJhbmQgPGlzaGthbWllbEBnbWFpbC5jb20+DQo+ID4gU2lnbmVkLW9m
Zi1ieTogS2VlcyBDb29rIDxrZWVzY29va0BjaHJvbWl1bS5vcmc+DQo+ID4gU2lnbmVkLW9mZi1i
eTogRGF2aWQgV2luZHNvciA8ZHdpbmRzb3JAZ21haWwuY29tPg0KPiBbLi4uXQ0KPiA+IGRpZmYg
LS1naXQgYS9kcml2ZXJzL21lZGlhL3BjaS9jeDg4L2N4ODguaCBiL2RyaXZlcnMvbWVkaWEvcGNp
L2N4ODgvY3g4OC5oDQo+ID4gaW5kZXggMTE1NDE0Yy4uMTZjMTMxMyAxMDA2NDQNCj4gPiAtLS0g
YS9kcml2ZXJzL21lZGlhL3BjaS9jeDg4L2N4ODguaA0KPiA+ICsrKyBiL2RyaXZlcnMvbWVkaWEv
cGNpL2N4ODgvY3g4OC5oDQo+ID4gQEAgLTI0LDYgKzI0LDcgQEANCj4gPiAgI2luY2x1ZGUgPGxp
bnV4L2kyYy1hbGdvLWJpdC5oPg0KPiA+ICAjaW5jbHVkZSA8bGludXgvdmlkZW9kZXYyLmg+DQo+
ID4gICNpbmNsdWRlIDxsaW51eC9rZGV2X3QuaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L3JlZmNv
dW50Lmg+DQo+ID4NCj4gPiAgI2luY2x1ZGUgPG1lZGlhL3Y0bDItZGV2aWNlLmg+DQo+ID4gICNp
bmNsdWRlIDxtZWRpYS92NGwyLWZoLmg+DQo+ID4gQEAgLTMzOSw3ICszNDAsNyBAQCBzdHJ1Y3Qg
Y3g4ODAyX2RldjsNCj4gPg0KPiA+ICBzdHJ1Y3QgY3g4OF9jb3JlIHsNCj4gPiAgCXN0cnVjdCBs
aXN0X2hlYWQgICAgICAgICAgIGRldmxpc3Q7DQo+ID4gLQlhdG9taWNfdCAgICAgICAgICAgICAg
ICAgICByZWZjb3VudDsNCj4gPiArCXJlZmNvdW50X3QgICAgICAgICAgICAgICAgICAgcmVmY291
bnQ7DQo+IA0KPiAgICAgQ291bGQgeW91IHBsZWFzZSBrZWVwIHRoZSBuYW1lIGFsaWduZWQgd2l0
aCBhYm92ZSBhbmQgYmVsb3c/DQoNCllvdSBtZWFuICJub3QgYWxpZ25lZCIgdG8gZGV2bGlzdCwg
YnV0IHdpdGggYSBzaGlmdCBsaWtlIGl0IHdhcyBiZWZvcmU/IA0KU3VyZSwgd2lsbCBmaXguIElz
IHRoZSBwYXRjaCBvayBvdGhlcndpc2U/DQoNCkJlc3QgUmVnYXJkcywNCkVsZW5hLg0KDQo+IA0K
PiA+DQo+ID4gIAkvKiBib2FyZCBuYW1lICovDQo+ID4gIAlpbnQgICAgICAgICAgICAgICAgICAg
ICAgICBucjsNCj4gPg0KPiANCj4gTUJSLCBTZXJnZWkNCg0K
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sakari Ailus March 7, 2017, 8:22 a.m. UTC | #3
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>
Sergei Shtylyov March 7, 2017, 10:40 a.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;