Groups > linux.kernel > #1593407 > unrolled thread

[PATCH 00/29] drivers, mics refcount conversions

Started by	Elena Reshetova <elena.reshetova@intel.com>
First post	2017-03-06 15:30 +0100
Last post	2017-03-06 16:30 +0100
Articles	20 on this page of 52 — 12 participants

Back to article view | Back to linux.kernel

  [PATCH 00/29] drivers, mics refcount conversions Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
    [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
      Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to  refcount_t Shaohua Li <shli@kernel.org> - 2017-03-07 20:20 +0100
        RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t  to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-08 10:50 +0100
          Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to  refcount_t "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org> - 2017-03-08 11:30 +0100
    [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
      Re: [PATCH 10/29] drivers, md: convert stripe_head.count from  atomic_t to refcount_t Shaohua Li <shli@kernel.org> - 2017-03-07 20:20 +0100
        RE: [PATCH 10/29] drivers, md: convert stripe_head.count from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-08 10:50 +0100
    [PATCH 26/29] drivers, usb: convert dev_data.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
    [PATCH 06/29] drivers, md: convert dm_cache_metadata.ref_count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
    [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
      Re: [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t  to refcount_t "Nicholas A. Bellinger" <nab@linux-iscsi.org> - 2017-03-08 08:50 +0100
    [PATCH 02/29] drivers, firewire: convert fw_node.ref_count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:30 +0100
    [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
      Re: [PATCH 13/29] drivers, media: convert  vb2_vmarea_handler.refcount from atomic_t to refcount_t Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-07 10:00 +0100
        RE: [PATCH 13/29] drivers, media: convert  vb2_vmarea_handler.refcount from atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-07 16:00 +0100
    [PATCH 16/29] drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 15/29] drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
      Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from  atomic_t to refcount_t Chris Leech <cleech@redhat.com> - 2017-03-08 19:50 +0100
        RE: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-09 08:30 +0100
          Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from  atomic_t to refcount_t Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-09 09:50 +0100
            RE: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-09 10:30 +0100
              Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from  atomic_t to refcount_t Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-09 10:40 +0100
    [PATCH 27/29] drivers, usb: convert ep_data.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 19/29] drivers, s390: convert lcs_reply.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
      Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from  atomic_t to refcount_t Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-06 16:30 +0100
        Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from  atomic_t to refcount_t Benjamin Block <bblock@linux.vnet.ibm.com> - 2017-03-06 20:50 +0100
        RE: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-07 12:20 +0100
        RE: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from  atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-08 15:00 +0100
          Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from  atomic_t to refcount_t Johannes Thumshirn <jthumshirn@suse.de> - 2017-03-08 16:30 +0100
    [PATCH 23/29] drivers: convert vme_user_vma_priv.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 14/29] drivers, media: convert vb2_dc_buf.refcount from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 04/29] drivers, connector: convert cn_callback_entry.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 05/29] drivers, md, bcache: convert cached_dev.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 17/29] drivers, pci: convert hv_pci_dev.refs from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
      Re: [PATCH 17/29] drivers, pci: convert hv_pci_dev.refs from  atomic_t to refcount_t Bjorn Helgaas <helgaas@kernel.org> - 2017-03-06 23:30 +0100
        Re: [PATCH 17/29] drivers, pci: convert hv_pci_dev.refs from  atomic_t to refcount_t Stephen Hemminger <stephen@networkplumber.org> - 2017-03-07 21:00 +0100
    [PATCH 20/29] drivers, s390: convert qeth_reply.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 07/29] drivers, md: convert dm_dev_internal.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 29/29] drivers, xen: convert grant_map.users from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
      Re: [Xen-devel] [PATCH 29/29] drivers, xen: convert grant_map.users  from atomic_t to refcount_t Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-03-06 19:50 +0100
        RE: [Xen-devel] [PATCH 29/29] drivers, xen: convert grant_map.users  from atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-08 15:10 +0100
          Re: [Xen-devel] [PATCH 29/29] drivers, xen: convert grant_map.users  from atomic_t to refcount_t Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-03-08 18:50 +0100
            RE: [Xen-devel] [PATCH 29/29] drivers, xen: convert grant_map.users  from atomic_t to refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-03-09 08:30 +0100
    [PATCH 25/29] drivers, usb: convert ffs_data.ref from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 28/29] drivers: convert sbd_duart.map_guard from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:40 +0100
    [PATCH 09/29] drivers, md: convert table_device.count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:50 +0100
    [PATCH 01/29] drivers, block: convert xen_blkif.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:50 +0100
    [PATCH 18/29] drivers, s390: convert urdev.ref_count from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 15:50 +0100
    [PATCH 03/29] drivers, char: convert vma_data.refcnt from atomic_t to refcount_t Elena Reshetova <elena.reshetova@intel.com> - 2017-03-06 16:30 +0100

Page 1 of 3 [1] 2 3 Next page →

#1593407 — [PATCH 00/29] drivers, mics refcount conversions

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 00/29] drivers, mics refcount conversions
Message-ID	<ti1O9-Yn-3@gated-at.bofh.it>

This series, for various different drivers, replaces atomic_t reference
counters with the new refcount_t type and API (see include/linux/refcount.h).
By doing this we prevent intentional or accidental
underflows or overflows that can led to use-after-free vulnerabilities.

The below patches are fully independent and can be cherry-picked separately*.
Since we convert all kernel subsystems in the same fashion, resulting
in about 300 patches, we have to group them for sending at least in some
fashion to be manageable. Please excuse the long cc list.

*with the exception of the media/vb2-related patches that depend on
vb2_vmarea_handler.refcount conversions.

Not run-time tested beyond booting and using kernel with refcount conversions
for my daily work.

If there are no objections to these patches,
I think they can go via Greg's drivers tree, as he suggested before.

Elena Reshetova (29):
  drivers, block: convert xen_blkif.refcnt from atomic_t to refcount_t
  drivers, firewire: convert fw_node.ref_count from atomic_t to
    refcount_t
  drivers, char: convert vma_data.refcnt from atomic_t to refcount_t
  drivers, connector: convert cn_callback_entry.refcnt from atomic_t to
    refcount_t
  drivers, md, bcache: convert cached_dev.count from atomic_t to
    refcount_t
  drivers, md: convert dm_cache_metadata.ref_count from atomic_t to
    refcount_t
  drivers, md: convert dm_dev_internal.count from atomic_t to refcount_t
  drivers, md: convert mddev.active from atomic_t to refcount_t
  drivers, md: convert table_device.count from atomic_t to refcount_t
  drivers, md: convert stripe_head.count from atomic_t to refcount_t
  drivers, media: convert cx88_core.refcount from atomic_t to refcount_t
  drivers, media: convert s2255_dev.num_channels from atomic_t to
    refcount_t
  drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to
    refcount_t
  drivers, media: convert vb2_dc_buf.refcount from atomic_t to
    refcount_t
  drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to
    refcount_t
  drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to
    refcount_t
  drivers, pci: convert hv_pci_dev.refs from atomic_t to refcount_t
  drivers, s390: convert urdev.ref_count from atomic_t to refcount_t
  drivers, s390: convert lcs_reply.refcnt from atomic_t to refcount_t
  drivers, s390: convert qeth_reply.refcnt from atomic_t to refcount_t
  drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t
  drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t
  drivers: convert vme_user_vma_priv.refcnt from atomic_t to refcount_t
  drivers: convert iblock_req.pending from atomic_t to refcount_t
  drivers, usb: convert ffs_data.ref from atomic_t to refcount_t
  drivers, usb: convert dev_data.count from atomic_t to refcount_t
  drivers, usb: convert ep_data.count from atomic_t to refcount_t
  drivers: convert sbd_duart.map_guard from atomic_t to refcount_t
  drivers, xen: convert grant_map.users from atomic_t to refcount_t

 drivers/block/xen-blkback/common.h             |  7 +--
 drivers/block/xen-blkback/xenbus.c             |  2 +-
 drivers/char/mspec.c                           |  9 ++--
 drivers/connector/cn_queue.c                   |  4 +-
 drivers/connector/connector.c                  |  2 +-
 drivers/firewire/core-topology.c               |  2 +-
 drivers/firewire/core.h                        |  8 ++--
 drivers/md/bcache/bcache.h                     |  7 +--
 drivers/md/bcache/super.c                      |  6 +--
 drivers/md/bcache/writeback.h                  |  2 +-
 drivers/md/dm-cache-metadata.c                 |  9 ++--
 drivers/md/dm-table.c                          |  6 +--
 drivers/md/dm.c                                | 12 +++--
 drivers/md/dm.h                                |  3 +-
 drivers/md/md.c                                |  6 +--
 drivers/md/md.h                                |  3 +-
 drivers/md/raid5-cache.c                       |  8 ++--
 drivers/md/raid5.c                             | 66 +++++++++++++-------------
 drivers/md/raid5.h                             |  3 +-
 drivers/media/pci/cx88/cx88-cards.c            |  2 +-
 drivers/media/pci/cx88/cx88-core.c             |  4 +-
 drivers/media/pci/cx88/cx88.h                  |  3 +-
 drivers/media/usb/s2255/s2255drv.c             | 21 ++++----
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 11 +++--
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 11 +++--
 drivers/media/v4l2-core/videobuf2-memops.c     |  6 +--
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 11 +++--
 drivers/pci/host/pci-hyperv.c                  |  9 ++--
 drivers/s390/char/vmur.c                       |  8 ++--
 drivers/s390/char/vmur.h                       |  4 +-
 drivers/s390/net/lcs.c                         |  8 ++--
 drivers/s390/net/lcs.h                         |  3 +-
 drivers/s390/net/qeth_core.h                   |  3 +-
 drivers/s390/net/qeth_core_main.c              |  8 ++--
 drivers/scsi/libfc/fc_fcp.c                    |  6 +--
 drivers/scsi/libiscsi.c                        |  8 ++--
 drivers/scsi/qedi/qedi_iscsi.c                 |  2 +-
 drivers/staging/vme/devices/vme_user.c         | 10 ++--
 drivers/target/target_core_iblock.c            | 12 ++---
 drivers/target/target_core_iblock.h            |  3 +-
 drivers/tty/serial/sb1250-duart.c              | 18 +++----
 drivers/usb/gadget/function/f_fs.c             |  8 ++--
 drivers/usb/gadget/function/u_fs.h             |  3 +-
 drivers/usb/gadget/legacy/inode.c              | 17 +++----
 drivers/xen/gntdev.c                           | 11 +++--
 include/linux/connector.h                      |  4 +-
 include/media/videobuf2-memops.h               |  3 +-
 include/scsi/libfc.h                           |  3 +-
 include/scsi/libiscsi.h                        |  3 +-
 49 files changed, 203 insertions(+), 185 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1593408 — [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
Message-ID	<ti1O9-Yn-7@gated-at.bofh.it>
In reply to	#1593407

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/md/md.c | 6 +++---
 drivers/md/md.h | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 985374f..94c8ebf 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -449,7 +449,7 @@ EXPORT_SYMBOL(md_unplug);
 
 static inline struct mddev *mddev_get(struct mddev *mddev)
 {
-	atomic_inc(&mddev->active);
+	refcount_inc(&mddev->active);
 	return mddev;
 }
 
@@ -459,7 +459,7 @@ static void mddev_put(struct mddev *mddev)
 {
 	struct bio_set *bs = NULL;
 
-	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
+	if (!refcount_dec_and_lock(&mddev->active, &all_mddevs_lock))
 		return;
 	if (!mddev->raid_disks && list_empty(&mddev->disks) &&
 	    mddev->ctime == 0 && !mddev->hold_active) {
@@ -495,7 +495,7 @@ void mddev_init(struct mddev *mddev)
 	INIT_LIST_HEAD(&mddev->all_mddevs);
 	setup_timer(&mddev->safemode_timer, md_safemode_timeout,
 		    (unsigned long) mddev);
-	atomic_set(&mddev->active, 1);
+	refcount_set(&mddev->active, 1);
 	atomic_set(&mddev->openers, 0);
 	atomic_set(&mddev->active_io, 0);
 	spin_lock_init(&mddev->lock);
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b8859cb..4811663 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -22,6 +22,7 @@
 #include <linux/list.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
+#include <linux/refcount.h>
 #include <linux/timer.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
@@ -360,7 +361,7 @@ struct mddev {
 	 */
 	struct mutex			open_mutex;
 	struct mutex			reconfig_mutex;
-	atomic_t			active;		/* general refcount */
+	refcount_t			active;		/* general refcount */
 	atomic_t			openers;	/* number of active opens */
 
 	int				changed;	/* True if we might need to
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1594551 — Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

From	Shaohua Li <shli@kernel.org>
Date	2017-03-07 20:20 +0100
Subject	Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
Message-ID	<tisOl-3ze-1@gated-at.bofh.it>
In reply to	#1593408

On Mon, Mar 06, 2017 at 04:20:55PM +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.

Looks good. Let me know how do you want to route the patch to upstream.
 
> 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/md/md.c | 6 +++---
>  drivers/md/md.h | 3 ++-
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 985374f..94c8ebf 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -449,7 +449,7 @@ EXPORT_SYMBOL(md_unplug);
>  
>  static inline struct mddev *mddev_get(struct mddev *mddev)
>  {
> -	atomic_inc(&mddev->active);
> +	refcount_inc(&mddev->active);
>  	return mddev;
>  }
>  
> @@ -459,7 +459,7 @@ static void mddev_put(struct mddev *mddev)
>  {
>  	struct bio_set *bs = NULL;
>  
> -	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
> +	if (!refcount_dec_and_lock(&mddev->active, &all_mddevs_lock))
>  		return;
>  	if (!mddev->raid_disks && list_empty(&mddev->disks) &&
>  	    mddev->ctime == 0 && !mddev->hold_active) {
> @@ -495,7 +495,7 @@ void mddev_init(struct mddev *mddev)
>  	INIT_LIST_HEAD(&mddev->all_mddevs);
>  	setup_timer(&mddev->safemode_timer, md_safemode_timeout,
>  		    (unsigned long) mddev);
> -	atomic_set(&mddev->active, 1);
> +	refcount_set(&mddev->active, 1);
>  	atomic_set(&mddev->openers, 0);
>  	atomic_set(&mddev->active_io, 0);
>  	spin_lock_init(&mddev->lock);
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index b8859cb..4811663 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -22,6 +22,7 @@
>  #include <linux/list.h>
>  #include <linux/mm.h>
>  #include <linux/mutex.h>
> +#include <linux/refcount.h>
>  #include <linux/timer.h>
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
> @@ -360,7 +361,7 @@ struct mddev {
>  	 */
>  	struct mutex			open_mutex;
>  	struct mutex			reconfig_mutex;
> -	atomic_t			active;		/* general refcount */
> +	refcount_t			active;		/* general refcount */
>  	atomic_t			openers;	/* number of active opens */
>  
>  	int				changed;	/* True if we might need to
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1594990 — RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

From	"Reshetova, Elena" <elena.reshetova@intel.com>
Date	2017-03-08 10:50 +0100
Subject	RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
Message-ID	<tiGoi-4Ft-23@gated-at.bofh.it>
In reply to	#1594551

> On Mon, Mar 06, 2017 at 04:20:55PM +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.
> 
> Looks good. Let me know how do you want to route the patch to upstream.

Greg, you previously mentioned that driver's conversions can go via your tree. Does this still apply?
Or should I be asking maintainers to merge these patches via their trees? 
I am not sure about the correct (and easier for everyone) way, please suggest.  

Best Regards,
Elena.
> 
> > 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/md/md.c | 6 +++---
> >  drivers/md/md.h | 3 ++-
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 985374f..94c8ebf 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -449,7 +449,7 @@ EXPORT_SYMBOL(md_unplug);
> >
> >  static inline struct mddev *mddev_get(struct mddev *mddev)
> >  {
> > -	atomic_inc(&mddev->active);
> > +	refcount_inc(&mddev->active);
> >  	return mddev;
> >  }
> >
> > @@ -459,7 +459,7 @@ static void mddev_put(struct mddev *mddev)
> >  {
> >  	struct bio_set *bs = NULL;
> >
> > -	if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock))
> > +	if (!refcount_dec_and_lock(&mddev->active, &all_mddevs_lock))
> >  		return;
> >  	if (!mddev->raid_disks && list_empty(&mddev->disks) &&
> >  	    mddev->ctime == 0 && !mddev->hold_active) {
> > @@ -495,7 +495,7 @@ void mddev_init(struct mddev *mddev)
> >  	INIT_LIST_HEAD(&mddev->all_mddevs);
> >  	setup_timer(&mddev->safemode_timer, md_safemode_timeout,
> >  		    (unsigned long) mddev);
> > -	atomic_set(&mddev->active, 1);
> > +	refcount_set(&mddev->active, 1);
> >  	atomic_set(&mddev->openers, 0);
> >  	atomic_set(&mddev->active_io, 0);
> >  	spin_lock_init(&mddev->lock);
> > diff --git a/drivers/md/md.h b/drivers/md/md.h
> > index b8859cb..4811663 100644
> > --- a/drivers/md/md.h
> > +++ b/drivers/md/md.h
> > @@ -22,6 +22,7 @@
> >  #include <linux/list.h>
> >  #include <linux/mm.h>
> >  #include <linux/mutex.h>
> > +#include <linux/refcount.h>
> >  #include <linux/timer.h>
> >  #include <linux/wait.h>
> >  #include <linux/workqueue.h>
> > @@ -360,7 +361,7 @@ struct mddev {
> >  	 */
> >  	struct mutex			open_mutex;
> >  	struct mutex			reconfig_mutex;
> > -	atomic_t			active;
> 	/* general refcount */
> > +	refcount_t			active;
> 	/* general refcount */
> >  	atomic_t			openers;	/*
> number of active opens */
> >
> >  	int
> 	changed;	/* True if we might need to
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1595025 — Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

From	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Date	2017-03-08 11:30 +0100
Subject	Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
Message-ID	<tiH10-5e8-21@gated-at.bofh.it>
In reply to	#1594990

On Wed, Mar 08, 2017 at 09:42:09AM +0000, Reshetova, Elena wrote:
> > On Mon, Mar 06, 2017 at 04:20:55PM +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.
> > 
> > Looks good. Let me know how do you want to route the patch to upstream.
> 
> Greg, you previously mentioned that driver's conversions can go via your tree. Does this still apply?
> Or should I be asking maintainers to merge these patches via their trees? 

You should ask them to take them through their trees, if they have them.
I'll be glad to scoop up all of the remaining ones that get missed, or
for subsystems that do not have trees.

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1593410 — [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t
Message-ID	<ti1O9-Yn-13@gated-at.bofh.it>
In reply to	#1593407

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/md/raid5-cache.c |  8 +++---
 drivers/md/raid5.c       | 66 ++++++++++++++++++++++++------------------------
 drivers/md/raid5.h       |  3 ++-
 3 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 3f307be..6c05e12 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -979,7 +979,7 @@ int r5l_write_stripe(struct r5l_log *log, struct stripe_head *sh)
 	 * don't delay.
 	 */
 	clear_bit(STRIPE_DELAYED, &sh->state);
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 
 	mutex_lock(&log->io_mutex);
 	/* meta + data */
@@ -1321,7 +1321,7 @@ static void r5c_flush_stripe(struct r5conf *conf, struct stripe_head *sh)
 	assert_spin_locked(&conf->device_lock);
 
 	list_del_init(&sh->lru);
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 
 	set_bit(STRIPE_HANDLE, &sh->state);
 	atomic_inc(&conf->active_stripes);
@@ -1424,7 +1424,7 @@ static void r5c_do_reclaim(struct r5conf *conf)
 			 */
 			if (!list_empty(&sh->lru) &&
 			    !test_bit(STRIPE_HANDLE, &sh->state) &&
-			    atomic_read(&sh->count) == 0) {
+			    refcount_read(&sh->count) == 0) {
 				r5c_flush_stripe(conf, sh);
 				if (count++ >= R5C_RECLAIM_STRIPE_GROUP)
 					break;
@@ -2650,7 +2650,7 @@ r5c_cache_data(struct r5l_log *log, struct stripe_head *sh,
 	 * don't delay.
 	 */
 	clear_bit(STRIPE_DELAYED, &sh->state);
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 
 	mutex_lock(&log->io_mutex);
 	/* meta + data */
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 2ce23b0..30c96a8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -296,7 +296,7 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
 static void __release_stripe(struct r5conf *conf, struct stripe_head *sh,
 			     struct list_head *temp_inactive_list)
 {
-	if (atomic_dec_and_test(&sh->count))
+	if (refcount_dec_and_test(&sh->count))
 		do_release_stripe(conf, sh, temp_inactive_list);
 }
 
@@ -388,7 +388,7 @@ void raid5_release_stripe(struct stripe_head *sh)
 
 	/* Avoid release_list until the last reference.
 	 */
-	if (atomic_add_unless(&sh->count, -1, 1))
+	if (refcount_dec_not_one(&sh->count))
 		return;
 
 	if (unlikely(!conf->mddev->thread) ||
@@ -401,7 +401,7 @@ void raid5_release_stripe(struct stripe_head *sh)
 slow_path:
 	local_irq_save(flags);
 	/* we are ok here if STRIPE_ON_RELEASE_LIST is set or not */
-	if (atomic_dec_and_lock(&sh->count, &conf->device_lock)) {
+	if (refcount_dec_and_lock(&sh->count, &conf->device_lock)) {
 		INIT_LIST_HEAD(&list);
 		hash = sh->hash_lock_index;
 		do_release_stripe(conf, sh, &list);
@@ -491,7 +491,7 @@ static void init_stripe(struct stripe_head *sh, sector_t sector, int previous)
 	struct r5conf *conf = sh->raid_conf;
 	int i, seq;
 
-	BUG_ON(atomic_read(&sh->count) != 0);
+	BUG_ON(refcount_read(&sh->count) != 0);
 	BUG_ON(test_bit(STRIPE_HANDLE, &sh->state));
 	BUG_ON(stripe_operations_active(sh));
 	BUG_ON(sh->batch_head);
@@ -668,11 +668,11 @@ raid5_get_active_stripe(struct r5conf *conf, sector_t sector,
 					  &conf->cache_state);
 			} else {
 				init_stripe(sh, sector, previous);
-				atomic_inc(&sh->count);
+				refcount_inc(&sh->count);
 			}
-		} else if (!atomic_inc_not_zero(&sh->count)) {
+		} else if (!refcount_inc_not_zero(&sh->count)) {
 			spin_lock(&conf->device_lock);
-			if (!atomic_read(&sh->count)) {
+			if (!refcount_read(&sh->count)) {
 				if (!test_bit(STRIPE_HANDLE, &sh->state))
 					atomic_inc(&conf->active_stripes);
 				BUG_ON(list_empty(&sh->lru) &&
@@ -688,7 +688,7 @@ raid5_get_active_stripe(struct r5conf *conf, sector_t sector,
 					sh->group = NULL;
 				}
 			}
-			atomic_inc(&sh->count);
+			refcount_inc(&sh->count);
 			spin_unlock(&conf->device_lock);
 		}
 	} while (sh == NULL);
@@ -752,9 +752,9 @@ static void stripe_add_to_batch_list(struct r5conf *conf, struct stripe_head *sh
 	hash = stripe_hash_locks_hash(head_sector);
 	spin_lock_irq(conf->hash_locks + hash);
 	head = __find_stripe(conf, head_sector, conf->generation);
-	if (head && !atomic_inc_not_zero(&head->count)) {
+	if (head && !refcount_inc_not_zero(&head->count)) {
 		spin_lock(&conf->device_lock);
-		if (!atomic_read(&head->count)) {
+		if (!refcount_read(&head->count)) {
 			if (!test_bit(STRIPE_HANDLE, &head->state))
 				atomic_inc(&conf->active_stripes);
 			BUG_ON(list_empty(&head->lru) &&
@@ -770,7 +770,7 @@ static void stripe_add_to_batch_list(struct r5conf *conf, struct stripe_head *sh
 				head->group = NULL;
 			}
 		}
-		atomic_inc(&head->count);
+		refcount_inc(&head->count);
 		spin_unlock(&conf->device_lock);
 	}
 	spin_unlock_irq(conf->hash_locks + hash);
@@ -833,7 +833,7 @@ static void stripe_add_to_batch_list(struct r5conf *conf, struct stripe_head *sh
 		sh->batch_head->bm_seq = seq;
 	}
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 unlock_out:
 	unlock_two_stripes(head, sh);
 out:
@@ -1036,9 +1036,9 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 			pr_debug("%s: for %llu schedule op %d on disc %d\n",
 				__func__, (unsigned long long)sh->sector,
 				bi->bi_opf, i);
-			atomic_inc(&sh->count);
+			refcount_inc(&sh->count);
 			if (sh != head_sh)
-				atomic_inc(&head_sh->count);
+				refcount_inc(&head_sh->count);
 			if (use_new_offset(conf, sh))
 				bi->bi_iter.bi_sector = (sh->sector
 						 + rdev->new_data_offset);
@@ -1097,9 +1097,9 @@ static void ops_run_io(struct stripe_head *sh, struct stripe_head_state *s)
 				 "replacement disc %d\n",
 				__func__, (unsigned long long)sh->sector,
 				rbi->bi_opf, i);
-			atomic_inc(&sh->count);
+			refcount_inc(&sh->count);
 			if (sh != head_sh)
-				atomic_inc(&head_sh->count);
+				refcount_inc(&head_sh->count);
 			if (use_new_offset(conf, sh))
 				rbi->bi_iter.bi_sector = (sh->sector
 						  + rrdev->new_data_offset);
@@ -1275,7 +1275,7 @@ static void ops_run_biofill(struct stripe_head *sh)
 		}
 	}
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 	init_async_submit(&submit, ASYNC_TX_ACK, tx, ops_complete_biofill, sh, NULL);
 	async_trigger_callback(&submit);
 }
@@ -1353,7 +1353,7 @@ ops_run_compute5(struct stripe_head *sh, struct raid5_percpu *percpu)
 		if (i != target)
 			xor_srcs[count++] = sh->dev[i].page;
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 
 	init_async_submit(&submit, ASYNC_TX_FENCE|ASYNC_TX_XOR_ZERO_DST, NULL,
 			  ops_complete_compute, sh, to_addr_conv(sh, percpu, 0));
@@ -1441,7 +1441,7 @@ ops_run_compute6_1(struct stripe_head *sh, struct raid5_percpu *percpu)
 	BUG_ON(!test_bit(R5_Wantcompute, &tgt->flags));
 	dest = tgt->page;
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 
 	if (target == qd_idx) {
 		count = set_syndrome_sources(blocks, sh, SYNDROME_SRC_ALL);
@@ -1516,7 +1516,7 @@ ops_run_compute6_2(struct stripe_head *sh, struct raid5_percpu *percpu)
 	pr_debug("%s: stripe: %llu faila: %d failb: %d\n",
 		 __func__, (unsigned long long)sh->sector, faila, failb);
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 
 	if (failb == syndrome_disks+1) {
 		/* Q disk is one of the missing disks */
@@ -1784,7 +1784,7 @@ ops_run_reconstruct5(struct stripe_head *sh, struct raid5_percpu *percpu,
 			break;
 	}
 	if (i >= sh->disks) {
-		atomic_inc(&sh->count);
+		refcount_inc(&sh->count);
 		set_bit(R5_Discard, &sh->dev[pd_idx].flags);
 		ops_complete_reconstruct(sh);
 		return;
@@ -1825,7 +1825,7 @@ ops_run_reconstruct5(struct stripe_head *sh, struct raid5_percpu *percpu,
 		flags = ASYNC_TX_ACK |
 			(prexor ? ASYNC_TX_XOR_DROP_DST : ASYNC_TX_XOR_ZERO_DST);
 
-		atomic_inc(&head_sh->count);
+		refcount_inc(&head_sh->count);
 		init_async_submit(&submit, flags, tx, ops_complete_reconstruct, head_sh,
 				  to_addr_conv(sh, percpu, j));
 	} else {
@@ -1867,7 +1867,7 @@ ops_run_reconstruct6(struct stripe_head *sh, struct raid5_percpu *percpu,
 			break;
 	}
 	if (i >= sh->disks) {
-		atomic_inc(&sh->count);
+		refcount_inc(&sh->count);
 		set_bit(R5_Discard, &sh->dev[sh->pd_idx].flags);
 		set_bit(R5_Discard, &sh->dev[sh->qd_idx].flags);
 		ops_complete_reconstruct(sh);
@@ -1891,7 +1891,7 @@ ops_run_reconstruct6(struct stripe_head *sh, struct raid5_percpu *percpu,
 				 struct stripe_head, batch_list) == head_sh;
 
 	if (last_stripe) {
-		atomic_inc(&head_sh->count);
+		refcount_inc(&head_sh->count);
 		init_async_submit(&submit, txflags, tx, ops_complete_reconstruct,
 				  head_sh, to_addr_conv(sh, percpu, j));
 	} else
@@ -1948,7 +1948,7 @@ static void ops_run_check_p(struct stripe_head *sh, struct raid5_percpu *percpu)
 	tx = async_xor_val(xor_dest, xor_srcs, 0, count, STRIPE_SIZE,
 			   &sh->ops.zero_sum_result, &submit);
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 	init_async_submit(&submit, ASYNC_TX_ACK, tx, ops_complete_check, sh, NULL);
 	tx = async_trigger_callback(&submit);
 }
@@ -1967,7 +1967,7 @@ static void ops_run_check_pq(struct stripe_head *sh, struct raid5_percpu *percpu
 	if (!checkp)
 		srcs[count] = NULL;
 
-	atomic_inc(&sh->count);
+	refcount_inc(&sh->count);
 	init_async_submit(&submit, ASYNC_TX_ACK, NULL, ops_complete_check,
 			  sh, to_addr_conv(sh, percpu, 0));
 	async_syndrome_val(srcs, 0, count+2, STRIPE_SIZE,
@@ -2057,7 +2057,7 @@ static struct stripe_head *alloc_stripe(struct kmem_cache *sc, gfp_t gfp,
 		INIT_LIST_HEAD(&sh->lru);
 		INIT_LIST_HEAD(&sh->r5c);
 		INIT_LIST_HEAD(&sh->log_list);
-		atomic_set(&sh->count, 1);
+		refcount_set(&sh->count, 1);
 		sh->log_start = MaxSector;
 		for (i = 0; i < disks; i++) {
 			struct r5dev *dev = &sh->dev[i];
@@ -2354,7 +2354,7 @@ static int drop_one_stripe(struct r5conf *conf)
 	spin_unlock_irq(conf->hash_locks + hash);
 	if (!sh)
 		return 0;
-	BUG_ON(atomic_read(&sh->count));
+	BUG_ON(refcount_read(&sh->count));
 	shrink_buffers(sh);
 	kmem_cache_free(conf->slab_cache, sh);
 	atomic_dec(&conf->active_stripes);
@@ -2386,7 +2386,7 @@ static void raid5_end_read_request(struct bio * bi)
 			break;
 
 	pr_debug("end_read_request %llu/%d, count: %d, error %d.\n",
-		(unsigned long long)sh->sector, i, atomic_read(&sh->count),
+		(unsigned long long)sh->sector, i, refcount_read(&sh->count),
 		bi->bi_error);
 	if (i == disks) {
 		bio_reset(bi);
@@ -2523,7 +2523,7 @@ static void raid5_end_write_request(struct bio *bi)
 		}
 	}
 	pr_debug("end_write_request %llu/%d, count %d, error: %d.\n",
-		(unsigned long long)sh->sector, i, atomic_read(&sh->count),
+		(unsigned long long)sh->sector, i, refcount_read(&sh->count),
 		bi->bi_error);
 	if (i == disks) {
 		bio_reset(bi);
@@ -4545,7 +4545,7 @@ static void handle_stripe(struct stripe_head *sh)
 	pr_debug("handling stripe %llu, state=%#lx cnt=%d, "
 		"pd_idx=%d, qd_idx=%d\n, check:%d, reconstruct:%d\n",
 	       (unsigned long long)sh->sector, sh->state,
-	       atomic_read(&sh->count), sh->pd_idx, sh->qd_idx,
+	       refcount_read(&sh->count), sh->pd_idx, sh->qd_idx,
 	       sh->check_state, sh->reconstruct_state);
 
 	analyse_stripe(sh, &s);
@@ -4924,7 +4924,7 @@ static void activate_bit_delay(struct r5conf *conf,
 		struct stripe_head *sh = list_entry(head.next, struct stripe_head, lru);
 		int hash;
 		list_del_init(&sh->lru);
-		atomic_inc(&sh->count);
+		refcount_inc(&sh->count);
 		hash = sh->hash_lock_index;
 		__release_stripe(conf, sh, &temp_inactive_list[hash]);
 	}
@@ -5240,7 +5240,7 @@ static struct stripe_head *__get_priority_stripe(struct r5conf *conf, int group)
 		sh->group = NULL;
 	}
 	list_del_init(&sh->lru);
-	BUG_ON(atomic_inc_return(&sh->count) != 1);
+	BUG_ON(refcount_inc_not_zero(&sh->count));
 	return sh;
 }
 
diff --git a/drivers/md/raid5.h b/drivers/md/raid5.h
index 4bb27b9..a1ed351 100644
--- a/drivers/md/raid5.h
+++ b/drivers/md/raid5.h
@@ -3,6 +3,7 @@
 
 #include <linux/raid/xor.h>
 #include <linux/dmaengine.h>
+#include <linux/refcount.h>
 
 /*
  *
@@ -207,7 +208,7 @@ struct stripe_head {
 	short			ddf_layout;/* use DDF ordering to calculate Q */
 	short			hash_lock_index;
 	unsigned long		state;		/* state flags */
-	atomic_t		count;	      /* nr of active thread/requests */
+	refcount_t		count;	      /* nr of active thread/requests */
 	int			bm_seq;	/* sequence number for bitmap flushes */
 	int			disks;		/* disks in stripe */
 	int			overwrite_disks; /* total overwrite disks in stripe,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1594562 — Re: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

From	Shaohua Li <shli@kernel.org>
Date	2017-03-07 20:20 +0100
Subject	Re: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t
Message-ID	<tisOm-3ze-29@gated-at.bofh.it>
In reply to	#1593410

On Mon, Mar 06, 2017 at 04:20:57PM +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>
> ---
>  drivers/md/raid5-cache.c |  8 +++---
>  drivers/md/raid5.c       | 66 ++++++++++++++++++++++++------------------------
>  drivers/md/raid5.h       |  3 ++-
>  3 files changed, 39 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 3f307be..6c05e12 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c

snip
>  	       sh->check_state, sh->reconstruct_state);
>  
>  	analyse_stripe(sh, &s);
> @@ -4924,7 +4924,7 @@ static void activate_bit_delay(struct r5conf *conf,
>  		struct stripe_head *sh = list_entry(head.next, struct stripe_head, lru);
>  		int hash;
>  		list_del_init(&sh->lru);
> -		atomic_inc(&sh->count);
> +		refcount_inc(&sh->count);
>  		hash = sh->hash_lock_index;
>  		__release_stripe(conf, sh, &temp_inactive_list[hash]);
>  	}
> @@ -5240,7 +5240,7 @@ static struct stripe_head *__get_priority_stripe(struct r5conf *conf, int group)
>  		sh->group = NULL;
>  	}
>  	list_del_init(&sh->lru);
> -	BUG_ON(atomic_inc_return(&sh->count) != 1);
> +	BUG_ON(refcount_inc_not_zero(&sh->count));

This changes the behavior. refcount_inc_not_zero doesn't inc if original value is 0

Thanks,
Shaohua

[toc] | [prev] | [next] | [standalone]

#1594988 — RE: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

From	"Reshetova, Elena" <elena.reshetova@intel.com>
Date	2017-03-08 10:50 +0100
Subject	RE: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t
Message-ID	<tiGoi-4Ft-5@gated-at.bofh.it>
In reply to	#1594562

> On Mon, Mar 06, 2017 at 04:20:57PM +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>
> > ---
> >  drivers/md/raid5-cache.c |  8 +++---
> >  drivers/md/raid5.c       | 66 ++++++++++++++++++++++++------------------------
> >  drivers/md/raid5.h       |  3 ++-
> >  3 files changed, 39 insertions(+), 38 deletions(-)
> >
> > diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> > index 3f307be..6c05e12 100644
> > --- a/drivers/md/raid5-cache.c
> > +++ b/drivers/md/raid5-cache.c
> 
> snip
> >  	       sh->check_state, sh->reconstruct_state);
> >
> >  	analyse_stripe(sh, &s);
> > @@ -4924,7 +4924,7 @@ static void activate_bit_delay(struct r5conf *conf,
> >  		struct stripe_head *sh = list_entry(head.next, struct
> stripe_head, lru);
> >  		int hash;
> >  		list_del_init(&sh->lru);
> > -		atomic_inc(&sh->count);
> > +		refcount_inc(&sh->count);
> >  		hash = sh->hash_lock_index;
> >  		__release_stripe(conf, sh,
> &temp_inactive_list[hash]);
> >  	}
> > @@ -5240,7 +5240,7 @@ static struct stripe_head *__get_priority_stripe(struct
> r5conf *conf, int group)
> >  		sh->group = NULL;
> >  	}
> >  	list_del_init(&sh->lru);
> > -	BUG_ON(atomic_inc_return(&sh->count) != 1);
> > +	BUG_ON(refcount_inc_not_zero(&sh->count));
> 
> This changes the behavior. refcount_inc_not_zero doesn't inc if original value is 0

Hm.. So, you want to inc here in any case and BUG if the end result differs from 1. 
So essentially you want to only increment here from zero to one under normal conditions... This is a challenge for refcount_t and against the design.
Is it ok just to maybe do this here:

-	BUG_ON(atomic_inc_return(&sh->count) != 1);
+	BUG_ON(refcount_read(&sh->count) != 0);
+	refcount_set((&sh->count, 1);

Do we have an issue with locking in this case? Or maybe it is then better to leave this one to be atomic_t without protection since it isn't a real refcounter as it turns out. 

Best Regards,
Elena. 

> 
> Thanks,
> Shaohua

[toc] | [prev] | [next] | [standalone]

#1593412 — [PATCH 26/29] drivers, usb: convert dev_data.count from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 26/29] drivers, usb: convert dev_data.count from atomic_t to refcount_t
Message-ID	<ti1O9-Yn-19@gated-at.bofh.it>
In reply to	#1593407

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/usb/gadget/legacy/inode.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 79a2d8f..81d76f3 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -27,6 +27,7 @@
 #include <linux/mmu_context.h>
 #include <linux/aio.h>
 #include <linux/uio.h>
+#include <linux/refcount.h>
 
 #include <linux/device.h>
 #include <linux/moduleparam.h>
@@ -114,7 +115,7 @@ enum ep0_state {
 
 struct dev_data {
 	spinlock_t			lock;
-	atomic_t			count;
+	refcount_t			count;
 	enum ep0_state			state;		/* P: lock */
 	struct usb_gadgetfs_event	event [N_EVENT];
 	unsigned			ev_next;
@@ -150,12 +151,12 @@ struct dev_data {
 
 static inline void get_dev (struct dev_data *data)
 {
-	atomic_inc (&data->count);
+	refcount_inc (&data->count);
 }
 
 static void put_dev (struct dev_data *data)
 {
-	if (likely (!atomic_dec_and_test (&data->count)))
+	if (likely (!refcount_dec_and_test (&data->count)))
 		return;
 	/* needs no more cleanup */
 	BUG_ON (waitqueue_active (&data->wait));
@@ -170,7 +171,7 @@ static struct dev_data *dev_new (void)
 	if (!dev)
 		return NULL;
 	dev->state = STATE_DEV_DISABLED;
-	atomic_set (&dev->count, 1);
+	refcount_set (&dev->count, 1);
 	spin_lock_init (&dev->lock);
 	INIT_LIST_HEAD (&dev->epfiles);
 	init_waitqueue_head (&dev->wait);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1593414 — [PATCH 06/29] drivers, md: convert dm_cache_metadata.ref_count from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 06/29] drivers, md: convert dm_cache_metadata.ref_count from atomic_t to refcount_t
Message-ID	<ti1Oa-Yn-23@gated-at.bofh.it>
In reply to	#1593407

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/md/dm-cache-metadata.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-cache-metadata.c b/drivers/md/dm-cache-metadata.c
index e4c2c1a..6d26e71 100644
--- a/drivers/md/dm-cache-metadata.c
+++ b/drivers/md/dm-cache-metadata.c
@@ -13,6 +13,7 @@
 #include "persistent-data/dm-transaction-manager.h"
 
 #include <linux/device-mapper.h>
+#include <linux/refcount.h>
 
 /*----------------------------------------------------------------*/
 
@@ -102,7 +103,7 @@ struct cache_disk_superblock {
 } __packed;
 
 struct dm_cache_metadata {
-	atomic_t ref_count;
+	refcount_t ref_count;
 	struct list_head list;
 
 	unsigned version;
@@ -756,7 +757,7 @@ static struct dm_cache_metadata *metadata_open(struct block_device *bdev,
 	}
 
 	cmd->version = metadata_version;
-	atomic_set(&cmd->ref_count, 1);
+	refcount_set(&cmd->ref_count, 1);
 	init_rwsem(&cmd->root_lock);
 	cmd->bdev = bdev;
 	cmd->data_block_size = data_block_size;
@@ -794,7 +795,7 @@ static struct dm_cache_metadata *lookup(struct block_device *bdev)
 
 	list_for_each_entry(cmd, &table, list)
 		if (cmd->bdev == bdev) {
-			atomic_inc(&cmd->ref_count);
+			refcount_inc(&cmd->ref_count);
 			return cmd;
 		}
 
@@ -865,7 +866,7 @@ struct dm_cache_metadata *dm_cache_metadata_open(struct block_device *bdev,
 
 void dm_cache_metadata_close(struct dm_cache_metadata *cmd)
 {
-	if (atomic_dec_and_test(&cmd->ref_count)) {
+	if (refcount_dec_and_test(&cmd->ref_count)) {
 		mutex_lock(&table_lock);
 		list_del(&cmd->list);
 		mutex_unlock(&table_lock);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1593415 — [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t
Message-ID	<ti1Oa-Yn-25@gated-at.bofh.it>
In reply to	#1593407

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/target/target_core_iblock.c | 12 ++++++------
 drivers/target/target_core_iblock.h |  3 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index d316ed5..bb069eb 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -279,7 +279,7 @@ static void iblock_complete_cmd(struct se_cmd *cmd)
 	struct iblock_req *ibr = cmd->priv;
 	u8 status;
 
-	if (!atomic_dec_and_test(&ibr->pending))
+	if (!refcount_dec_and_test(&ibr->pending))
 		return;
 
 	if (atomic_read(&ibr->ib_bio_err_cnt))
@@ -487,7 +487,7 @@ iblock_execute_write_same(struct se_cmd *cmd)
 	bio_list_init(&list);
 	bio_list_add(&list, bio);
 
-	atomic_set(&ibr->pending, 1);
+	refcount_set(&ibr->pending, 1);
 
 	while (sectors) {
 		while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset)
@@ -498,7 +498,7 @@ iblock_execute_write_same(struct se_cmd *cmd)
 			if (!bio)
 				goto fail_put_bios;
 
-			atomic_inc(&ibr->pending);
+			refcount_inc(&ibr->pending);
 			bio_list_add(&list, bio);
 		}
 
@@ -706,7 +706,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	cmd->priv = ibr;
 
 	if (!sgl_nents) {
-		atomic_set(&ibr->pending, 1);
+		refcount_set(&ibr->pending, 1);
 		iblock_complete_cmd(cmd);
 		return 0;
 	}
@@ -719,7 +719,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	bio_list_init(&list);
 	bio_list_add(&list, bio);
 
-	atomic_set(&ibr->pending, 2);
+	refcount_set(&ibr->pending, 2);
 	bio_cnt = 1;
 
 	for_each_sg(sgl, sg, sgl_nents, i) {
@@ -740,7 +740,7 @@ iblock_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 			if (!bio)
 				goto fail_put_bios;
 
-			atomic_inc(&ibr->pending);
+			refcount_inc(&ibr->pending);
 			bio_list_add(&list, bio);
 			bio_cnt++;
 		}
diff --git a/drivers/target/target_core_iblock.h b/drivers/target/target_core_iblock.h
index 718d3fc..f2a5797 100644
--- a/drivers/target/target_core_iblock.h
+++ b/drivers/target/target_core_iblock.h
@@ -2,6 +2,7 @@
 #define TARGET_CORE_IBLOCK_H
 
 #include <linux/atomic.h>
+#include <linux/refcount.h>
 #include <target/target_core_base.h>
 
 #define IBLOCK_VERSION		"4.0"
@@ -10,7 +11,7 @@
 #define IBLOCK_LBA_SHIFT	9
 
 struct iblock_req {
-	atomic_t pending;
+	refcount_t pending;
 	atomic_t ib_bio_err_cnt;
 } ____cacheline_aligned;
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1594910 — Re: [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t

From	"Nicholas A. Bellinger" <nab@linux-iscsi.org>
Date	2017-03-08 08:50 +0100
Subject	Re: [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t
Message-ID	<tiEw9-3fU-9@gated-at.bofh.it>
In reply to	#1593415

Hi Elena,

On Mon, 2017-03-06 at 16:21 +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>
> ---
>  drivers/target/target_core_iblock.c | 12 ++++++------
>  drivers/target/target_core_iblock.h |  3 ++-
>  2 files changed, 8 insertions(+), 7 deletions(-)

For the target_core_iblock part:

Acked-by: Nicholas Bellinger <nab@linux-iscsi.org>

[toc] | [prev] | [next] | [standalone]

#1593416 — [PATCH 02/29] drivers, firewire: convert fw_node.ref_count from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:30 +0100
Subject	[PATCH 02/29] drivers, firewire: convert fw_node.ref_count from atomic_t to refcount_t
Message-ID	<ti1Oa-Yn-33@gated-at.bofh.it>
In reply to	#1593407

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/firewire/core-topology.c | 2 +-
 drivers/firewire/core.h          | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 0de8350..939d259 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -124,7 +124,7 @@ static struct fw_node *fw_node_create(u32 sid, int port_count, int color)
 	node->initiated_reset = SELF_ID_PHY_INITIATOR(sid);
 	node->port_count = port_count;
 
-	atomic_set(&node->ref_count, 1);
+	refcount_set(&node->ref_count, 1);
 	INIT_LIST_HEAD(&node->link);
 
 	return node;
diff --git a/drivers/firewire/core.h b/drivers/firewire/core.h
index e1480ff6..c07962e 100644
--- a/drivers/firewire/core.h
+++ b/drivers/firewire/core.h
@@ -12,7 +12,7 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
-#include <linux/atomic.h>
+#include <linux/refcount.h>
 
 struct device;
 struct fw_card;
@@ -184,7 +184,7 @@ struct fw_node {
 			 * local node to this node. */
 	u8 max_depth:4;	/* Maximum depth to any leaf node */
 	u8 max_hops:4;	/* Max hops in this sub tree */
-	atomic_t ref_count;
+	refcount_t ref_count;
 
 	/* For serializing node topology into a list. */
 	struct list_head link;
@@ -197,14 +197,14 @@ struct fw_node {
 
 static inline struct fw_node *fw_node_get(struct fw_node *node)
 {
-	atomic_inc(&node->ref_count);
+	refcount_inc(&node->ref_count);
 
 	return node;
 }
 
 static inline void fw_node_put(struct fw_node *node)
 {
-	if (atomic_dec_and_test(&node->ref_count))
+	if (refcount_dec_and_test(&node->ref_count))
 		kfree(node);
 }
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1593417 — [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:40 +0100
Subject	[PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t
Message-ID	<ti1XP-146-3@gated-at.bofh.it>
In reply to	#1593407

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/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;
 	void			(*put)(void *arg);
 	void			*arg;
 };
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1594010 — Re: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t

From	Sakari Ailus <sakari.ailus@iki.fi>
Date	2017-03-07 10:00 +0100
Subject	Re: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t
Message-ID	<tij8m-4ZD-37@gated-at.bofh.it>
In reply to	#1593417

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@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/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.

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

>  	void			(*put)(void *arg);
>  	void			*arg;
>  };

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1594315 — RE: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t

From	"Reshetova, Elena" <elena.reshetova@intel.com>
Date	2017-03-07 16:00 +0100
Subject	RE: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t
Message-ID	<tioKJ-w2-3@gated-at.bofh.it>
In reply to	#1594010

> 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@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/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@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]

#1593418 — [PATCH 16/29] drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:40 +0100
Subject	[PATCH 16/29] drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to refcount_t
Message-ID	<ti1XP-146-1@gated-at.bofh.it>
In reply to	#1593407

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/v4l2-core/videobuf2-vmalloc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 3f77814..f83253a 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -13,6 +13,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/mm.h>
+#include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/vmalloc.h>
@@ -26,7 +27,7 @@ struct vb2_vmalloc_buf {
 	struct frame_vector		*vec;
 	enum dma_data_direction		dma_dir;
 	unsigned long			size;
-	atomic_t			refcount;
+	refcount_t			refcount;
 	struct vb2_vmarea_handler	handler;
 	struct dma_buf			*dbuf;
 };
@@ -56,7 +57,7 @@ static void *vb2_vmalloc_alloc(struct device *dev, unsigned long attrs,
 		return ERR_PTR(-ENOMEM);
 	}
 
-	atomic_inc(&buf->refcount);
+	refcount_set(&buf->refcount, 1);
 	return buf;
 }
 
@@ -64,7 +65,7 @@ static void vb2_vmalloc_put(void *buf_priv)
 {
 	struct vb2_vmalloc_buf *buf = buf_priv;
 
-	if (atomic_dec_and_test(&buf->refcount)) {
+	if (refcount_dec_and_test(&buf->refcount)) {
 		vfree(buf->vaddr);
 		kfree(buf);
 	}
@@ -161,7 +162,7 @@ static void *vb2_vmalloc_vaddr(void *buf_priv)
 static unsigned int vb2_vmalloc_num_users(void *buf_priv)
 {
 	struct vb2_vmalloc_buf *buf = buf_priv;
-	return atomic_read(&buf->refcount);
+	return refcount_read(&buf->refcount);
 }
 
 static int vb2_vmalloc_mmap(void *buf_priv, struct vm_area_struct *vma)
@@ -368,7 +369,7 @@ static struct dma_buf *vb2_vmalloc_get_dmabuf(void *buf_priv, unsigned long flag
 		return NULL;
 
 	/* dmabuf keeps reference to vb2 buffer */
-	atomic_inc(&buf->refcount);
+	refcount_inc(&buf->refcount);
 
 	return dbuf;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1593419 — [PATCH 15/29] drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:40 +0100
Subject	[PATCH 15/29] drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to refcount_t
Message-ID	<ti1XP-146-5@gated-at.bofh.it>
In reply to	#1593407

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/v4l2-core/videobuf2-dma-sg.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index ecff8f4..29fde1a 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -12,6 +12,7 @@
 
 #include <linux/module.h>
 #include <linux/mm.h>
+#include <linux/refcount.h>
 #include <linux/scatterlist.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
@@ -46,7 +47,7 @@ struct vb2_dma_sg_buf {
 	struct sg_table			*dma_sgt;
 	size_t				size;
 	unsigned int			num_pages;
-	atomic_t			refcount;
+	refcount_t			refcount;
 	struct vb2_vmarea_handler	handler;
 
 	struct dma_buf_attachment	*db_attach;
@@ -150,7 +151,7 @@ static void *vb2_dma_sg_alloc(struct device *dev, unsigned long dma_attrs,
 	buf->handler.put = vb2_dma_sg_put;
 	buf->handler.arg = buf;
 
-	atomic_inc(&buf->refcount);
+	refcount_set(&buf->refcount, 1);
 
 	dprintk(1, "%s: Allocated buffer of %d pages\n",
 		__func__, buf->num_pages);
@@ -176,7 +177,7 @@ static void vb2_dma_sg_put(void *buf_priv)
 	struct sg_table *sgt = &buf->sg_table;
 	int i = buf->num_pages;
 
-	if (atomic_dec_and_test(&buf->refcount)) {
+	if (refcount_dec_and_test(&buf->refcount)) {
 		dprintk(1, "%s: Freeing buffer of %d pages\n", __func__,
 			buf->num_pages);
 		dma_unmap_sg_attrs(buf->dev, sgt->sgl, sgt->orig_nents,
@@ -320,7 +321,7 @@ static unsigned int vb2_dma_sg_num_users(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
 
-	return atomic_read(&buf->refcount);
+	return refcount_read(&buf->refcount);
 }
 
 static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
@@ -530,7 +531,7 @@ static struct dma_buf *vb2_dma_sg_get_dmabuf(void *buf_priv, unsigned long flags
 		return NULL;
 
 	/* dmabuf keeps reference to vb2 buffer */
-	atomic_inc(&buf->refcount);
+	refcount_inc(&buf->refcount);
 
 	return dbuf;
 }
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1593421 — [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

From	Elena Reshetova <elena.reshetova@intel.com>
Date	2017-03-06 15:40 +0100
Subject	[PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t
Message-ID	<ti1XP-146-9@gated-at.bofh.it>
In reply to	#1593407

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/scsi/libiscsi.c        | 8 ++++----
 drivers/scsi/qedi/qedi_iscsi.c | 2 +-
 include/scsi/libiscsi.h        | 3 ++-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 834d121..7eb1d2c 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -516,13 +516,13 @@ static void iscsi_free_task(struct iscsi_task *task)
 
 void __iscsi_get_task(struct iscsi_task *task)
 {
-	atomic_inc(&task->refcount);
+	refcount_inc(&task->refcount);
 }
 EXPORT_SYMBOL_GPL(__iscsi_get_task);
 
 void __iscsi_put_task(struct iscsi_task *task)
 {
-	if (atomic_dec_and_test(&task->refcount))
+	if (refcount_dec_and_test(&task->refcount))
 		iscsi_free_task(task);
 }
 EXPORT_SYMBOL_GPL(__iscsi_put_task);
@@ -744,7 +744,7 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
 	 * released by the lld when it has transmitted the task for
 	 * pdus we do not expect a response for.
 	 */
-	atomic_set(&task->refcount, 1);
+	refcount_set(&task->refcount, 1);
 	task->conn = conn;
 	task->sc = NULL;
 	INIT_LIST_HEAD(&task->running);
@@ -1616,7 +1616,7 @@ static inline struct iscsi_task *iscsi_alloc_task(struct iscsi_conn *conn,
 	sc->SCp.phase = conn->session->age;
 	sc->SCp.ptr = (char *) task;
 
-	atomic_set(&task->refcount, 1);
+	refcount_set(&task->refcount, 1);
 	task->state = ISCSI_TASK_PENDING;
 	task->conn = conn;
 	task->sc = sc;
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index b9f79d3..3895bd5 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -1372,7 +1372,7 @@ static void qedi_cleanup_task(struct iscsi_task *task)
 {
 	if (!task->sc || task->state == ISCSI_TASK_PENDING) {
 		QEDI_INFO(NULL, QEDI_LOG_IO, "Returning ref_cnt=%d\n",
-			  atomic_read(&task->refcount));
+			  refcount_read(&task->refcount));
 		return;
 	}
 
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index b0e275d..24d74b5 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -29,6 +29,7 @@
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 #include <linux/kfifo.h>
+#include <linux/refcount.h>
 #include <scsi/iscsi_proto.h>
 #include <scsi/iscsi_if.h>
 #include <scsi/scsi_transport_iscsi.h>
@@ -139,7 +140,7 @@ struct iscsi_task {
 
 	/* state set/tested under session->lock */
 	int			state;
-	atomic_t		refcount;
+	refcount_t		refcount;
 	struct list_head	running;	/* running cmd list */
 	void			*dd_data;	/* driver/transport data */
 };
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1595442 — Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

From	Chris Leech <cleech@redhat.com>
Date	2017-03-08 19:50 +0100
Subject	Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t
Message-ID	<tiOOS-1ZF-11@gated-at.bofh.it>
In reply to	#1593421

On Mon, Mar 06, 2017 at 04:21:09PM +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>

This looks OK to me.

Acked-by: Chris Leech <cleech@redhat.com>

> ---
>  drivers/scsi/libiscsi.c        | 8 ++++----
>  drivers/scsi/qedi/qedi_iscsi.c | 2 +-
>  include/scsi/libiscsi.h        | 3 ++-
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 834d121..7eb1d2c 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -516,13 +516,13 @@ static void iscsi_free_task(struct iscsi_task *task)
>  
>  void __iscsi_get_task(struct iscsi_task *task)
>  {
> -	atomic_inc(&task->refcount);
> +	refcount_inc(&task->refcount);
>  }
>  EXPORT_SYMBOL_GPL(__iscsi_get_task);
>  
>  void __iscsi_put_task(struct iscsi_task *task)
>  {
> -	if (atomic_dec_and_test(&task->refcount))
> +	if (refcount_dec_and_test(&task->refcount))
>  		iscsi_free_task(task);
>  }
>  EXPORT_SYMBOL_GPL(__iscsi_put_task);
> @@ -744,7 +744,7 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
>  	 * released by the lld when it has transmitted the task for
>  	 * pdus we do not expect a response for.
>  	 */
> -	atomic_set(&task->refcount, 1);
> +	refcount_set(&task->refcount, 1);
>  	task->conn = conn;
>  	task->sc = NULL;
>  	INIT_LIST_HEAD(&task->running);
> @@ -1616,7 +1616,7 @@ static inline struct iscsi_task *iscsi_alloc_task(struct iscsi_conn *conn,
>  	sc->SCp.phase = conn->session->age;
>  	sc->SCp.ptr = (char *) task;
>  
> -	atomic_set(&task->refcount, 1);
> +	refcount_set(&task->refcount, 1);
>  	task->state = ISCSI_TASK_PENDING;
>  	task->conn = conn;
>  	task->sc = sc;
> diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
> index b9f79d3..3895bd5 100644
> --- a/drivers/scsi/qedi/qedi_iscsi.c
> +++ b/drivers/scsi/qedi/qedi_iscsi.c
> @@ -1372,7 +1372,7 @@ static void qedi_cleanup_task(struct iscsi_task *task)
>  {
>  	if (!task->sc || task->state == ISCSI_TASK_PENDING) {
>  		QEDI_INFO(NULL, QEDI_LOG_IO, "Returning ref_cnt=%d\n",
> -			  atomic_read(&task->refcount));
> +			  refcount_read(&task->refcount));
>  		return;
>  	}
>  
> diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
> index b0e275d..24d74b5 100644
> --- a/include/scsi/libiscsi.h
> +++ b/include/scsi/libiscsi.h
> @@ -29,6 +29,7 @@
>  #include <linux/timer.h>
>  #include <linux/workqueue.h>
>  #include <linux/kfifo.h>
> +#include <linux/refcount.h>
>  #include <scsi/iscsi_proto.h>
>  #include <scsi/iscsi_if.h>
>  #include <scsi/scsi_transport_iscsi.h>
> @@ -139,7 +140,7 @@ struct iscsi_task {
>  
>  	/* state set/tested under session->lock */
>  	int			state;
> -	atomic_t		refcount;
> +	refcount_t		refcount;
>  	struct list_head	running;	/* running cmd list */
>  	void			*dd_data;	/* driver/transport data */
>  };
> -- 
> 2.7.4
>

[toc] | [prev] | [next] | [standalone]

Page 1 of 3 [1] 2 3 Next page →

csiph-web

[PATCH 00/29] drivers, mics refcount conversions

Contents

#1593407 — [PATCH 00/29] drivers, mics refcount conversions

#1593408 — [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

#1594551 — Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

#1594990 — RE: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

#1595025 — Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t

#1593410 — [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

#1594562 — Re: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

#1594988 — RE: [PATCH 10/29] drivers, md: convert stripe_head.count from atomic_t to refcount_t

#1593412 — [PATCH 26/29] drivers, usb: convert dev_data.count from atomic_t to refcount_t

#1593414 — [PATCH 06/29] drivers, md: convert dm_cache_metadata.ref_count from atomic_t to refcount_t

#1593415 — [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t

#1594910 — Re: [PATCH 24/29] drivers: convert iblock_req.pending from atomic_t to refcount_t

#1593416 — [PATCH 02/29] drivers, firewire: convert fw_node.ref_count from atomic_t to refcount_t

#1593417 — [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t

#1594010 — Re: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t

#1594315 — RE: [PATCH 13/29] drivers, media: convert vb2_vmarea_handler.refcount from atomic_t to refcount_t

#1593418 — [PATCH 16/29] drivers, media: convert vb2_vmalloc_buf.refcount from atomic_t to refcount_t

#1593419 — [PATCH 15/29] drivers, media: convert vb2_dma_sg_buf.refcount from atomic_t to refcount_t

#1593421 — [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

#1595442 — Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t