| Message ID | 1488810076-3754-22-git-send-email-elena.reshetova@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 03/06/2017 03:21 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. The subject is wrong, should be something like "scsi: libfc convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390. Other than that Acked-by: Johannes Thumshirn <jth@kernel.org>
On Mon, Mar 06, 2017 at 04:27:11PM +0100, Johannes Thumshirn wrote: > On 03/06/2017 03:21 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. > > The subject is wrong, should be something like "scsi: libfc convert > fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390. > Yes please, I was extremely confused for a moment here. Beste Grüße / Best regards, - Benjamin Block
> On 03/06/2017 03:21 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. > > The subject is wrong, should be something like "scsi: libfc convert > fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390. Very sorry about this: the error in the subject got from the time when I was trying to break the bigger drivers patch set into per-variable part and trying to automate the process too much :( I will fix it in the end version! Best Regards, Elena > > Other than that > Acked-by: Johannes Thumshirn <jth@kernel.org> > > -- > Johannes Thumshirn Storage > jthumshirn@suse.de +49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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
> On 03/06/2017 03:21 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. > > The subject is wrong, should be something like "scsi: libfc convert > fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390. > > Other than that > Acked-by: Johannes Thumshirn <jth@kernel.org> Turns out that it is better that all these patches go through the respective maintainer trees, if present. If I send an updated patch (with subject fixed), could you merge it through your tree? Best Regards, Elena. > > -- > Johannes Thumshirn Storage > jthumshirn@suse.de +49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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
On 03/08/2017 02:48 PM, Reshetova, Elena wrote: >> On 03/06/2017 03:21 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. >> >> The subject is wrong, should be something like "scsi: libfc convert >> fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390. >> >> Other than that >> Acked-by: Johannes Thumshirn <jth@kernel.org> > > Turns out that it is better that all these patches go through the respective maintainer trees, if present. > If I send an updated patch (with subject fixed), could you merge it through your tree? Yes, but this would be the normal scsi tree from Martin and James. Please include my Ack in the re-sends. Thanks a lot, Johannes
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 0e67621..a808e8e 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -154,7 +154,7 @@ static struct fc_fcp_pkt *fc_fcp_pkt_alloc(struct fc_lport *lport, gfp_t gfp) memset(fsp, 0, sizeof(*fsp)); fsp->lp = lport; fsp->xfer_ddp = FC_XID_UNKNOWN; - atomic_set(&fsp->ref_cnt, 1); + refcount_set(&fsp->ref_cnt, 1); init_timer(&fsp->timer); fsp->timer.data = (unsigned long)fsp; INIT_LIST_HEAD(&fsp->list); @@ -175,7 +175,7 @@ static struct fc_fcp_pkt *fc_fcp_pkt_alloc(struct fc_lport *lport, gfp_t gfp) */ static void fc_fcp_pkt_release(struct fc_fcp_pkt *fsp) { - if (atomic_dec_and_test(&fsp->ref_cnt)) { + if (refcount_dec_and_test(&fsp->ref_cnt)) { struct fc_fcp_internal *si = fc_get_scsi_internal(fsp->lp); mempool_free(fsp, si->scsi_pkt_pool); @@ -188,7 +188,7 @@ static void fc_fcp_pkt_release(struct fc_fcp_pkt *fsp) */ static void fc_fcp_pkt_hold(struct fc_fcp_pkt *fsp) { - atomic_inc(&fsp->ref_cnt); + refcount_inc(&fsp->ref_cnt); } /** diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h index da5033d..2109844 100644 --- a/include/scsi/libfc.h +++ b/include/scsi/libfc.h @@ -23,6 +23,7 @@ #include <linux/timer.h> #include <linux/if.h> #include <linux/percpu.h> +#include <linux/refcount.h> #include <scsi/scsi_transport.h> #include <scsi/scsi_transport_fc.h> @@ -321,7 +322,7 @@ struct fc_seq_els_data { */ struct fc_fcp_pkt { spinlock_t scsi_pkt_lock; - atomic_t ref_cnt; + refcount_t ref_cnt; /* SCSI command and data transfer information */ u32 data_len;