[PATCH] virtio_scsi: remove unnecessary condition check
kmem_cache_destroy()/mempool_destroy() can handle NULL pointer
correctly, so there is no need to check NULL pointer before calling
kmem_cache_destroy()/mempool_destroy().
Signed-off-by: Chengguang Xu
---
drivers/scsi/virtio_scsi.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 1c72db94270e..c8c92fb5910e 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -1012,14 +1012,11 @@ static int __init init(void)
return 0;
error:
- if (virtscsi_cmd_pool) {
- mempool_destroy(virtscsi_cmd_pool);
- virtscsi_cmd_pool = NULL;
- }
- if (virtscsi_cmd_cache) {
- kmem_cache_destroy(virtscsi_cmd_cache);
- virtscsi_cmd_cache = NULL;
- }
+ mempool_destroy(virtscsi_cmd_pool);
+ virtscsi_cmd_pool = NULL;
+ kmem_cache_destroy(virtscsi_cmd_cache);
+ virtscsi_cmd_cache = NULL;
+
return ret;
}
--
2.17.1
[PATCH] libfc: remove unnecessary condition check
kmem_cache_destroy() can handle NULL pointer correctly, so there is
no need to check NULL pointer before calling kmem_cache_destroy()
Signed-off-by: Chengguang Xu
---
drivers/scsi/libfc/fc_fcp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 4fae253d4f3d..563247def49a 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -2295,8 +2295,7 @@ int fc_setup_fcp(void)
void fc_destroy_fcp(void)
{
- if (scsi_pkt_cachep)
- kmem_cache_destroy(scsi_pkt_cachep);
+ kmem_cache_destroy(scsi_pkt_cachep);
}
/**
--
2.17.1
[PATCH] aic94xx: remove unnecessary condition check
kmem_cache_destroy() can handle NULL pointer correctly, so there is
no need to check NULL pointer before calling kmem_cache_destroy().
Signed-off-by: Chengguang Xu
---
drivers/scsi/aic94xx/aic94xx_init.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_init.c
b/drivers/scsi/aic94xx/aic94xx_init.c
index 1391e5f35918..06a2642b977a 100644
--- a/drivers/scsi/aic94xx/aic94xx_init.c
+++ b/drivers/scsi/aic94xx/aic94xx_init.c
@@ -660,12 +660,10 @@ static int asd_create_global_caches(void)
static void asd_destroy_global_caches(void)
{
- if (asd_dma_token_cache)
- kmem_cache_destroy(asd_dma_token_cache);
+ kmem_cache_destroy(asd_dma_token_cache);
asd_dma_token_cache = NULL;
- if (asd_ascb_cache)
- kmem_cache_destroy(asd_ascb_cache);
+ kmem_cache_destroy(asd_ascb_cache);
asd_ascb_cache = NULL;
}
--
2.17.1
Re: [PATCH 1/2] iscsi target: Set conn->sess to NULL when iscsi_login_set_conn_values fails
On Mon, Aug 27, 2018 at 02:45:15PM -0500, Mike Christie wrote: > Signed-off-by: Vincent Pelletier > [rebased against idr/ida changes and to handle ret review comments from > Matthew] > Signed-off-by: Mike Christie > Cc: Matthew Wilcox Reviewed-by: Matthew Wilcox
[PATCH 1/2] iscsi target: Set conn->sess to NULL when iscsi_login_set_conn_values fails
From: Vincent Pelletier
Fixes a use-after-free reported by KASAN when later
iscsi_target_login_sess_out gets called and it tries to access
conn->sess->se_sess:
Disabling lock debugging due to kernel taint
iSCSI Login timeout on Network Portal [::]:3260
iSCSI Login negotiation failed.
==
BUG: KASAN: use-after-free in
iscsi_target_login_sess_out.cold.12+0x58/0xff [iscsi_target_mod]
Read of size 8 at addr 880109d070c8 by task iscsi_np/980
CPU: 1 PID: 980 Comm: iscsi_np Tainted: G O
4.17.8kasan.sess.connops+ #4
Hardware name: To be filled by O.E.M. To be filled by O.E.M./Aptio CRB,
BIOS 5.6.5 05/19/2014
Call Trace:
dump_stack+0x71/0xac
print_address_description+0x65/0x22e
? iscsi_target_login_sess_out.cold.12+0x58/0xff [iscsi_target_mod]
kasan_report.cold.6+0x241/0x2fd
iscsi_target_login_sess_out.cold.12+0x58/0xff [iscsi_target_mod]
iscsi_target_login_thread+0x1086/0x1710 [iscsi_target_mod]
? __sched_text_start+0x8/0x8
? iscsi_target_login_sess_out+0x250/0x250 [iscsi_target_mod]
? __kthread_parkme+0xcc/0x100
? parse_args.cold.14+0xd3/0xd3
? iscsi_target_login_sess_out+0x250/0x250 [iscsi_target_mod]
kthread+0x1a0/0x1c0
? kthread_bind+0x30/0x30
ret_from_fork+0x35/0x40
Allocated by task 980:
kasan_kmalloc+0xbf/0xe0
kmem_cache_alloc_trace+0x112/0x210
iscsi_target_login_thread+0x816/0x1710 [iscsi_target_mod]
kthread+0x1a0/0x1c0
ret_from_fork+0x35/0x40
Freed by task 980:
__kasan_slab_free+0x125/0x170
kfree+0x90/0x1d0
iscsi_target_login_thread+0x1577/0x1710 [iscsi_target_mod]
kthread+0x1a0/0x1c0
ret_from_fork+0x35/0x40
The buggy address belongs to the object at 880109d06f00
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 456 bytes inside of
512-byte region [880109d06f00, 880109d07100)
The buggy address belongs to the page:
page:ea0004274180 count:1 mapcount:0 mapping:
index:0x0 compound_mapcount: 0
flags: 0x17fffc08100(slab|head)
raw: 017fffc08100 0001000c000c
raw: dead0100 dead0200 88011b002e00
page dumped because: kasan: bad access detected
Memory state around the buggy address:
880109d06f80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
880109d07000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>880109d07080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
880109d07100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
880109d07180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==
Signed-off-by: Vincent Pelletier
[rebased against idr/ida changes and to handle ret review comments from Matthew]
Signed-off-by: Mike Christie
Cc: Matthew Wilcox
---
v2:
- Rebase against Linus's tree with idr/ida changes.
- Always return -ENOMEM on failure because caller does not care and
to simplify the code.
drivers/target/iscsi/iscsi_target_login.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target_login.c
b/drivers/target/iscsi/iscsi_target_login.c
index 9e74f8b..f58b9c1 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -310,11 +310,9 @@ static int iscsi_login_zero_tsih_s1(
return -ENOMEM;
}
- ret = iscsi_login_set_conn_values(sess, conn, pdu->cid);
- if (unlikely(ret)) {
- kfree(sess);
- return ret;
- }
+ if (iscsi_login_set_conn_values(sess, conn, pdu->cid))
+ goto free_sess;
+
sess->init_task_tag = pdu->itt;
memcpy(>isid, pdu->isid, 6);
sess->exp_cmd_sn= be32_to_cpu(pdu->cmdsn);
--
2.7.2
[PATCH 0/2] iscsi target: login fixes
The following patches are login fixes from the list. The set has been rebased against Linus's current tree so they apply over the idr/ida patches that have been merged.
[PATCH 2/2] iscsi target: fix conn_ops double free
If iscsi_login_init_conn fails it can free conn_ops.
__iscsi_target_login_thread will then call iscsi_target_login_sess_out
which will also free it.
This fixes the problem by organizing conn allocation/setup into parts
that are needed through the life of the conn and parts that are only
needed for the login. The free functions then release what was allocated
in the alloc functions.
With this patch we have:
iscsit_alloc_conn/iscsit_free_conn - allocs/frees the conn we need
for the entire life of the conn.
iscsi_login_init_conn/iscsi_target_nego_release - allocs/frees the parts
of the conn that are only needed during login.
Signed-off-by: Mike Christie
---
drivers/target/iscsi/iscsi_target.c | 9 +-
drivers/target/iscsi/iscsi_target_login.c | 141 --
drivers/target/iscsi/iscsi_target_login.h | 2 +-
3 files changed, 77 insertions(+), 75 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target.c
b/drivers/target/iscsi/iscsi_target.c
index 94bad43..9cdfccb 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4208,22 +4208,15 @@ int iscsit_close_connection(
crypto_free_ahash(tfm);
}
- free_cpumask_var(conn->conn_cpumask);
-
- kfree(conn->conn_ops);
- conn->conn_ops = NULL;
-
if (conn->sock)
sock_release(conn->sock);
if (conn->conn_transport->iscsit_free_conn)
conn->conn_transport->iscsit_free_conn(conn);
- iscsit_put_transport(conn->conn_transport);
-
pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
conn->conn_state = TARG_CONN_STATE_FREE;
- kfree(conn);
+ iscsit_free_conn(conn);
spin_lock_bh(>conn_lock);
atomic_dec(>nconn);
diff --git a/drivers/target/iscsi/iscsi_target_login.c
b/drivers/target/iscsi/iscsi_target_login.c
index f58b9c1..bb90c80 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -67,45 +67,10 @@ static struct iscsi_login *iscsi_login_init_conn(struct
iscsi_conn *conn)
goto out_req_buf;
}
- conn->conn_ops = kzalloc(sizeof(struct iscsi_conn_ops), GFP_KERNEL);
- if (!conn->conn_ops) {
- pr_err("Unable to allocate memory for"
- " struct iscsi_conn_ops.\n");
- goto out_rsp_buf;
- }
-
- init_waitqueue_head(>queues_wq);
- INIT_LIST_HEAD(>conn_list);
- INIT_LIST_HEAD(>conn_cmd_list);
- INIT_LIST_HEAD(>immed_queue_list);
- INIT_LIST_HEAD(>response_queue_list);
- init_completion(>conn_post_wait_comp);
- init_completion(>conn_wait_comp);
- init_completion(>conn_wait_rcfr_comp);
- init_completion(>conn_waiting_on_uc_comp);
- init_completion(>conn_logout_comp);
- init_completion(>rx_half_close_comp);
- init_completion(>tx_half_close_comp);
- init_completion(>rx_login_comp);
- spin_lock_init(>cmd_lock);
- spin_lock_init(>conn_usage_lock);
- spin_lock_init(>immed_queue_lock);
- spin_lock_init(>nopin_timer_lock);
- spin_lock_init(>response_queue_lock);
- spin_lock_init(>state_lock);
-
- if (!zalloc_cpumask_var(>conn_cpumask, GFP_KERNEL)) {
- pr_err("Unable to allocate conn->conn_cpumask\n");
- goto out_conn_ops;
- }
conn->conn_login = login;
return login;
-out_conn_ops:
- kfree(conn->conn_ops);
-out_rsp_buf:
- kfree(login->rsp_buf);
out_req_buf:
kfree(login->req_buf);
out_login:
@@ -1147,6 +1112,75 @@ iscsit_conn_set_transport(struct iscsi_conn *conn,
struct iscsit_transport *t)
return 0;
}
+static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np)
+{
+ struct iscsi_conn *conn;
+
+ conn = kzalloc(sizeof(struct iscsi_conn), GFP_KERNEL);
+ if (!conn) {
+ pr_err("Could not allocate memory for new connection\n");
+ return NULL;
+ }
+ pr_debug("Moving to TARG_CONN_STATE_FREE.\n");
+ conn->conn_state = TARG_CONN_STATE_FREE;
+
+ init_waitqueue_head(>queues_wq);
+ INIT_LIST_HEAD(>conn_list);
+ INIT_LIST_HEAD(>conn_cmd_list);
+ INIT_LIST_HEAD(>immed_queue_list);
+ INIT_LIST_HEAD(>response_queue_list);
+ init_completion(>conn_post_wait_comp);
+ init_completion(>conn_wait_comp);
+ init_completion(>conn_wait_rcfr_comp);
+ init_completion(>conn_waiting_on_uc_comp);
+ init_completion(>conn_logout_comp);
+ init_completion(>rx_half_close_comp);
+ init_completion(>tx_half_close_comp);
+ init_completion(>rx_login_comp);
+ spin_lock_init(>cmd_lock);
+ spin_lock_init(>conn_usage_lock);
+ spin_lock_init(>immed_queue_lock);
+ spin_lock_init(>nopin_timer_lock);
+ spin_lock_init(>response_queue_lock);
+ spin_lock_init(>state_lock);
+
+
Re: [PATCH] scsi: aacraid: fix a signednes bug
Dan, > The problem is that ->reset_state is a u8 but it can be set to -1 or > -2 in aac_tmf_callback() and the error handling in > aac_eh_target_reset() relies on it to be signed. Applied to 4.19/scsi-fixes, thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 0/3]: iscsi target login fixes
Mike, > I will remake and resend the last 2 patches against the current Linus > tree, because the second patch does not cleanly apply against the > idr/ida changes. Sounds good. Just getting the new SCSI trees ready... -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 0/3]: iscsi target login fixes
On 08/27/2018 12:03 AM, Matthew Wilcox wrote: > On Fri, Aug 24, 2018 at 01:37:10PM -0500, Mike Christie wrote: >> The following patchset is a round up of login fixes that have been >> on the list and in Mathew's tree. They fix a couple of bugs in the >> iscsi login failure handling path. >> >> The patches were made against Martin's 4.19/scsi-queue branch. >> >> Matthew, the first patch is one that I had sent to you that had a fix >> for the idr issue you asked about on the list. There was a conflict so >> I had to update the patch. I was not sure if you wanted me to keep >> your signed off so to be safe I dropped it. >> >> Also, it sounded like your ida/idr patches were not going to make 4.19, >> so hopefully the idr iscsi login fix will be merged already when your >> patches get merged for 4.20, so you do not have to carry it anymore. > > Christmas came early, and Linus decided to pull the updated ida patches. > > commit 26abc916a898d34c5ad159315a2f683def3c > Author: Mike Christie > Date: Thu Jul 26 12:13:49 2018 -0500 > > iscsi target: fix session creation failure handling > > so patch 1 does not need to be added to v4.19/scsi-queue > Martin, I will remake and resend the last 2 patches against the current Linus tree, because the second patch does not cleanly apply against the idr/ida changes.
[PATCH] scsi: aacraid: fix a signednes bug
The problem is that ->reset_state is a u8 but it can be set to -1 or -2
in aac_tmf_callback() and the error handling in aac_eh_target_reset()
relies on it to be signed.
Fixes: 0d643ff3c353 ("scsi: aacraid: use aac_tmf_callback for reset fib")
Signed-off-by: Dan Carpenter
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 29bf1e60f542..39eb415987fc 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -1346,7 +1346,7 @@ struct fib {
struct aac_hba_map_info {
__le32 rmw_nexus; /* nexus for native HBA devices */
u8 devtype;/* device type */
- u8 reset_state;/* 0 - no reset, 1..x - */
+ s8 reset_state;/* 0 - no reset, 1..x - */
/* after xth TM LUN reset */
u16 qd_limit;
u32 scan_counter;
