On Mon, 1 September 2014 00:38:19 +0300, Sagi Grimberg wrote: > On 8/28/2014 4:01 AM, Joern Engel wrote: > >Found by coverity. l_conn cannot possible be NULL. It can very well > >not match any connection, which is the logical equivalent of NULL. At > >that point we would happily dereference the pointer and do heck knows > >what with the random values we find. A NULL pointer dereference would > >have been much better. > > > >Signed-off-by: Joern Engel <joern@xxxxxxxxx> > >--- > > drivers/target/iscsi/iscsi_target.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > >diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c > >index 30f4a7d42e32..f9d2b1255856 100644 > >--- a/drivers/target/iscsi/iscsi_target.c > >+++ b/drivers/target/iscsi/iscsi_target.c > >@@ -4549,7 +4549,7 @@ static void iscsit_logout_post_handler_diffcid( > > } > > spin_unlock_bh(&sess->conn_lock); > > > >- if (!l_conn) > >+ if (&l_conn->conn_list == &sess->sess_conn_list) > > I'm not sure I understand how can one get here even hypothetically. > Can you at least put a warn print here (or someone can sched some > light here...) That makes two of us. I am slightly concerned that some future code change can make it possible to trigger this condition. Therefore this patch plus a WARN_ON_ONCE seems preferrable to just removing the code. It is just a slight preference and if someone else strongly disagrees, I can live with that as well. > > return; > > > > if (l_conn->sock) > > > Jörn -- You can't tell where a program is going to spend its time. Bottlenecks occur in surprising places, so don't try to second guess and put in a speed hack until you've proven that's where the bottleneck is. -- Rob Pike -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
