[PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[Li Jinlin]

I got a kernel NULL pointer derference report as below:

    BUG: kernel NULL pointer dereference, address: 0000000000000038
    CPU: 4 PID: 1050 Comm: cat Not tainted 5.19.0 #38
    RIP: 0010:kernel_getpeername+0xf/0x30
    Call Trace:
     <TASK>
     ? iscsi_sw_tcp_conn_get_param+0x11f/0x17f
     show_conn_ep_param_ISCSI_PARAM_CONN_ADDRESS+0x90/0xb0
     dev_attr_show+0x1d/0x50
     sysfs_kf_seq_show+0xad/0x120
     kernfs_seq_show+0x2c/0x40
     seq_read_iter+0x12e/0x4d0
     ? aa_file_perm+0x177/0x590
     kernfs_fop_read_iter+0x183/0x210
     new_sync_read+0xfe/0x180
     ? 0xffffffff81000000
     vfs_read+0x14d/0x1a0
     ksys_read+0x6d/0xf0
     __x64_sys_read+0x1a/0x20
     do_syscall_64+0x3b/0x90
     entry_SYSCALL_64_after_hwframe+0x63/0xcd

The problem scenario is:

              CPU1                         CPU2
    -------------------------    ------------------------
    iscsi_sw_tcp_conn_get_param
      spin_lock_bh(&frwd_lock)
      if (!tcp_sw_conn || !tcp_sw_conn->sock)
         spin_unlock_bh(&frwd_lock)
         return -ENOTCONN

      sock = tcp_sw_conn->sock;
      sock_hold(sock->sk)
      spin_unlock_bh(&frwd_lock)
                                  iscsi_sw_tcp_release_conn
                                    spin_lock_bh(&frwd_lock)
                                    tcp_sw_conn->sock = NULL
                                    spin_unlock_bh(&frwd_lock)
                                    sockfd_put(sock)
                                      task_work
                                        __fput
                                          sock_close
                                            __sock_release
                                              sock->sk = NULL
                                              sock->ops = NULL
                                              sock->file = NULL
      kernel_getpeername
        sock->ops->getname
        ^^^^^^^^^
        NULL pointer dereference of sock->ops

sock_hold() and sock_put() can't ensure that sock_close() won't be
called before calling getpeername() or getsockname(). Using fget()
and sockfd_put() replace sock_hold() and sock_put(), and put them
under frwd_lock protection, to ensure that the socket struct is
preserved until after the getpeernaem() or getsockname() complete.

Fixes: bcf3a2953d36 ("scsi: iscsi: iscsi_tcp: Avoid holding spinlock while calling getpeername()")
Signed-off-by: Li Jinlin <lijinlin3@huawei.com>
---
 drivers/scsi/iscsi_tcp.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 9fee70d6434a..63532dc3970d 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -612,8 +612,8 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)
 
 	spin_lock_bh(&session->frwd_lock);
 	tcp_sw_conn->sock = NULL;
-	spin_unlock_bh(&session->frwd_lock);
 	sockfd_put(sock);
+	spin_unlock_bh(&session->frwd_lock);
 }
 
 static void iscsi_sw_tcp_conn_destroy(struct iscsi_cls_conn *cls_conn)
@@ -756,7 +756,7 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
 			return -ENOTCONN;
 		}
 		sock = tcp_sw_conn->sock;
-		sock_hold(sock->sk);
+		fget(sock->file);
 		spin_unlock_bh(&conn->session->frwd_lock);
 
 		if (param == ISCSI_PARAM_LOCAL_PORT)
@@ -765,7 +765,9 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
 		else
 			rc = kernel_getpeername(sock,
 						(struct sockaddr *)&addr);
-		sock_put(sock->sk);
+		spin_lock_bh(&conn->session->frwd_lock);
+		sockfd_put(sock);
+		spin_unlock_bh(&conn->session->frwd_lock);
 		if (rc < 0)
 			return rc;
 
@@ -808,12 +810,14 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host *shost,
 			spin_unlock_bh(&session->frwd_lock);
 			return -ENOTCONN;
 		}
-		sock_hold(sock->sk);
+		fget(sock->file);
 		spin_unlock_bh(&session->frwd_lock);
 
 		rc = kernel_getsockname(sock,
 					(struct sockaddr *)&addr);
-		sock_put(sock->sk);
+		spin_lock_bh(&conn->session->frwd_lock);
+		sockfd_put(sock);
+		spin_unlock_bh(&conn->session->frwd_lock);
 		if (rc < 0)
 			return rc;
 
-- 
2.27.0

RE: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[lijinlin (A)]

So sorry, this patch has problem, please ignore.

Thanks
Li Jinlin

Re: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[Mike Christie]

On 8/2/22 6:23 AM, lijinlin (A) wrote:
> So sorry, this patch has problem, please ignore.
> 

Was the issue the fget use?

I know I gave the suggestion to do the get, but seeing it now makes
me think I was wrong and it's getting too messy.

Let's just add a mutex for getting/setting the tcp_sw_conn->sock in
the non-io paths (io paths are flushed/locked already). Something like
this (patch is only compile tested):

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 9fee70d6434a..c1696472965e 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -558,6 +558,8 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session,
 	tcp_conn = conn->dd_data;
 	tcp_sw_conn = tcp_conn->dd_data;
 
+	mutex_init(&tcp_sw_conn->sock_lock);
+
 	tfm = crypto_alloc_ahash("crc32c", 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(tfm))
 		goto free_conn;
@@ -592,11 +594,15 @@ iscsi_sw_tcp_conn_create(struct iscsi_cls_session *cls_session,
 
 static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)
 {
-	struct iscsi_session *session = conn->session;
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
 	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
 	struct socket *sock = tcp_sw_conn->sock;
 
+	/*
+	 * The iscsi transport class will make sure we are not called in
+	 * parallel with start, stop, bind and destroys. However, this can be
+	 * called twice if userspace does a stop then a destroy.
+	 */
 	if (!sock)
 		return;
 
@@ -610,9 +616,9 @@ static void iscsi_sw_tcp_release_conn(struct iscsi_conn *conn)
 	iscsi_sw_tcp_conn_restore_callbacks(conn);
 	sock_put(sock->sk);
 
-	spin_lock_bh(&session->frwd_lock);
+	mutex_lock(&tcp_sw_conn->sock_lock);
 	tcp_sw_conn->sock = NULL;
-	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&tcp_sw_conn->sock_lock);
 	sockfd_put(sock);
 }
 
@@ -664,7 +670,6 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
 		       struct iscsi_cls_conn *cls_conn, uint64_t transport_eph,
 		       int is_leading)
 {
-	struct iscsi_session *session = cls_session->dd_data;
 	struct iscsi_conn *conn = cls_conn->dd_data;
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
 	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
@@ -684,10 +689,10 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
 	if (err)
 		goto free_socket;
 
-	spin_lock_bh(&session->frwd_lock);
+	mutex_lock(&tcp_sw_conn->sock_lock);
 	/* bind iSCSI connection and socket */
 	tcp_sw_conn->sock = sock;
-	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&tcp_sw_conn->sock_lock);
 
 	/* setup Socket parameters */
 	sk = sock->sk;
@@ -724,8 +729,15 @@ static int iscsi_sw_tcp_conn_set_param(struct iscsi_cls_conn *cls_conn,
 		break;
 	case ISCSI_PARAM_DATADGST_EN:
 		iscsi_set_param(cls_conn, param, buf, buflen);
+
+		mutex_lock(&tcp_sw_conn->sock_lock);
+		if (!tcp_sw_conn->sock) {
+			mutex_unlock(&tcp_sw_conn->sock_lock);
+			return -ENOTCONN;
+		}
 		tcp_sw_conn->sendpage = conn->datadgst_en ?
 			sock_no_sendpage : tcp_sw_conn->sock->ops->sendpage;
+		mutex_unlock(&tcp_sw_conn->sock_lock);
 		break;
 	case ISCSI_PARAM_MAX_R2T:
 		return iscsi_tcp_set_max_r2t(conn, buf);
@@ -750,14 +762,20 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
 	case ISCSI_PARAM_CONN_PORT:
 	case ISCSI_PARAM_CONN_ADDRESS:
 	case ISCSI_PARAM_LOCAL_PORT:
-		spin_lock_bh(&conn->session->frwd_lock);
-		if (!tcp_sw_conn || !tcp_sw_conn->sock) {
-			spin_unlock_bh(&conn->session->frwd_lock);
+		/*
+		 * The conn's sysfs interface is exposed to userspace after
+		 * the tcp_sw_conn is setup, and the netlink interface will
+		 * make sure we don't do a get_param while setup is running.
+		 * We do need to make sure a user is not accessing sysfs while
+		 * the netlink interface is releasing the sock via
+		 * iscsi_sw_tcp_release_conn.
+		 */
+		mutex_lock(&tcp_sw_conn->sock_lock);
+		sock = tcp_sw_conn->sock;
+		if (!sock) {
+			mutex_unlock(&tcp_sw_conn->sock_lock);
 			return -ENOTCONN;
 		}
-		sock = tcp_sw_conn->sock;
-		sock_hold(sock->sk);
-		spin_unlock_bh(&conn->session->frwd_lock);
 
 		if (param == ISCSI_PARAM_LOCAL_PORT)
 			rc = kernel_getsockname(sock,
@@ -765,7 +783,7 @@ static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
 		else
 			rc = kernel_getpeername(sock,
 						(struct sockaddr *)&addr);
-		sock_put(sock->sk);
+		mutex_unlock(&tcp_sw_conn->sock_lock);
 		if (rc < 0)
 			return rc;
 
@@ -803,17 +821,25 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host *shost,
 		}
 		tcp_conn = conn->dd_data;
 		tcp_sw_conn = tcp_conn->dd_data;
+		/*
+		 * If the leadconn is bound then setup has completed and destroy
+		 * has not been run yet. Grab a ref to the conn incase a destroy
+		 * starts to run after we drop the fwrd_lock.
+		 */
+		iscsi_get_conn(conn->cls_conn);
+		spin_unlock_bh(&session->frwd_lock);
+
+		mutex_lock(&tcp_sw_conn->sock_lock);
 		sock = tcp_sw_conn->sock;
 		if (!sock) {
-			spin_unlock_bh(&session->frwd_lock);
+			mutex_unlock(&tcp_sw_conn->sock_lock);
+			iscsi_put_conn(conn->cls_conn);
 			return -ENOTCONN;
 		}
-		sock_hold(sock->sk);
-		spin_unlock_bh(&session->frwd_lock);
-
-		rc = kernel_getsockname(sock,
-					(struct sockaddr *)&addr);
-		sock_put(sock->sk);
+	
+		rc = kernel_getsockname(sock, (struct sockaddr *)&addr);
+		mutex_unlock(&tcp_sw_conn->sock_lock);
+		iscsi_put_conn(conn->cls_conn);
 		if (rc < 0)
 			return rc;
 
diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
index 791453195099..7c6f90ce6124 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -28,6 +28,8 @@ struct iscsi_sw_tcp_send {
 
 struct iscsi_sw_tcp_conn {
 	struct socket		*sock;
+	/* Taken when accesing the sock from the netlink/sysfs interface */
+	struct mutex		sock_lock;
 
 	struct iscsi_sw_tcp_send out;
 	/* old values for socket callbacks */

Re: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[kernel test robot]

Hi Li,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v5.19 next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: loongarch-randconfig-s041-20220801 (https://download.01.org/0day-ci/archive/20220803/202208030633.x2jgVRIa-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/ccc367df3fdba07b24eeda721ca928cce50f40d2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
        git checkout ccc367df3fdba07b24eeda721ca928cce50f40d2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch SHELL=/bin/bash drivers/scsi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
   drivers/scsi/iscsi_tcp.c:798:26: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int fd @@     got struct file *file @@
   drivers/scsi/iscsi_tcp.c:798:26: sparse:     expected unsigned int fd
   drivers/scsi/iscsi_tcp.c:798:26: sparse:     got struct file *file
   drivers/scsi/iscsi_tcp.c:852:26: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int fd @@     got struct file *file @@
   drivers/scsi/iscsi_tcp.c:852:26: sparse:     expected unsigned int fd
   drivers/scsi/iscsi_tcp.c:852:26: sparse:     got struct file *file
>> drivers/scsi/iscsi_tcp.c:798:22: sparse: sparse: non size-preserving pointer to integer cast
   drivers/scsi/iscsi_tcp.c:852:22: sparse: sparse: non size-preserving pointer to integer cast

vim +798 drivers/scsi/iscsi_tcp.c

   777	
   778	static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
   779					       enum iscsi_param param, char *buf)
   780	{
   781		struct iscsi_conn *conn = cls_conn->dd_data;
   782		struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
   783		struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
   784		struct sockaddr_in6 addr;
   785		struct socket *sock;
   786		int rc;
   787	
   788		switch(param) {
   789		case ISCSI_PARAM_CONN_PORT:
   790		case ISCSI_PARAM_CONN_ADDRESS:
   791		case ISCSI_PARAM_LOCAL_PORT:
   792			spin_lock_bh(&conn->session->frwd_lock);
   793			if (!tcp_sw_conn || !tcp_sw_conn->sock) {
   794				spin_unlock_bh(&conn->session->frwd_lock);
   795				return -ENOTCONN;
   796			}
   797			sock = tcp_sw_conn->sock;
 > 798			fget(sock->file);
   799			spin_unlock_bh(&conn->session->frwd_lock);
   800	
   801			if (param == ISCSI_PARAM_LOCAL_PORT)
   802				rc = kernel_getsockname(sock,
   803							(struct sockaddr *)&addr);
   804			else
   805				rc = kernel_getpeername(sock,
   806							(struct sockaddr *)&addr);
   807			spin_lock_bh(&conn->session->frwd_lock);
   808			sockfd_put(sock);
   809			spin_unlock_bh(&conn->session->frwd_lock);
   810			if (rc < 0)
   811				return rc;
   812	
   813			return iscsi_conn_get_addr_param((struct sockaddr_storage *)
   814							 &addr, param, buf);
   815		default:
   816			return iscsi_conn_get_param(cls_conn, param, buf);
   817		}
   818	
   819		return 0;
   820	}
   821	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[Li Jinlin]

On 8/3/2022 12:25 AM, Mike Christie wrote:
> On 8/2/22 6:23 AM, lijinlin (A) wrote:
>> So sorry, this patch has problem, please ignore.
>>
> 
> Was the issue the fget use?
>> I know I gave the suggestion to do the get, but seeing it now makes
> me think I was wrong and it's getting too messy.
> 
I use get_file() in local, and test the patch can fix this null-ptr-deref.
But I got an INFO report as below, it only appears once in multiple
tests. I'm not sure if this info report represents a possible problem
with the patch. So I ask for ignore it.

    INFO: trying to register non-static key.
    The code is fine but needs lockdep annotation, or maybe
    you didn't initialize this object before use?
    turning off the locking correctness validator.
    CPU: 21 PID: 1074 Comm: cat Not tainted 5.19.0 #44
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014
    Call Trace:
    <TASK>
    dump_stack_lvl+0x49/0x63
    dump_stack+0x10/0x16
    register_lock_class+0x483/0x490
    ? reacquire_held_locks+0xcb/0x1e0
    ? release_sock+0x1e/0xb0
    __lock_acquire.constprop.0+0x4e/0x530
    ? lock_release+0x142/0x2d0
    lock_acquire+0xc3/0x1b0
    ? iscsi_sw_tcp_host_get_param+0xa4/0x120
    _raw_spin_lock_bh+0x34/0x50
    ? iscsi_sw_tcp_host_get_param+0xa4/0x120
    iscsi_sw_tcp_host_get_param+0xa4/0x120
    show_host_param_ISCSI_HOST_PARAM_IPADDRESS+0x56/0x70
    dev_attr_show+0x1d/0x50
    sysfs_kf_seq_show+0xad/0x120
    kernfs_seq_show+0x2c/0x40
    seq_read_iter+0x12e/0x4d0
    ? aa_file_perm+0x177/0x5a0
    kernfs_fop_read_iter+0x183/0x210
    new_sync_read+0xfe/0x180
    ? 0xffffffff81000000
    vfs_read+0x14d/0x1a0
    ksys_read+0x6d/0xf0
    __x64_sys_read+0x1a/0x20
    do_syscall_64+0x3b/0x90
    entry_SYSCALL_64_after_hwframe+0x63/0xcd


> Let's just add a mutex for getting/setting the tcp_sw_conn->sock in
> the non-io paths (io paths are flushed/locked already). Something like
> this (patch is only compile tested):
> 

This patch is clean, I have tested it and it is effective.
Please push this patch to the mainline, Thanks.

Jinlin

> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index 9fee70d6434a..c1696472965e 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c

Re: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[kernel test robot]

Hi Li,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v5.19 next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220803/202208032214.0FELL5gK-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/ccc367df3fdba07b24eeda721ca928cce50f40d2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
        git checkout ccc367df3fdba07b24eeda721ca928cce50f40d2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/scsi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/scsi/iscsi_tcp.c: In function 'iscsi_sw_tcp_conn_get_param':
>> drivers/scsi/iscsi_tcp.c:798:26: warning: passing argument 1 of 'fget' makes integer from pointer without a cast [-Wint-conversion]
     798 |                 fget(sock->file);
         |                      ~~~~^~~~~~
         |                          |
         |                          struct file *
   In file included from drivers/scsi/iscsi_tcp.c:25:
   include/linux/file.h:48:39: note: expected 'unsigned int' but argument is of type 'struct file *'
      48 | extern struct file *fget(unsigned int fd);
         |                          ~~~~~~~~~~~~~^~
   drivers/scsi/iscsi_tcp.c: In function 'iscsi_sw_tcp_host_get_param':
   drivers/scsi/iscsi_tcp.c:852:26: warning: passing argument 1 of 'fget' makes integer from pointer without a cast [-Wint-conversion]
     852 |                 fget(sock->file);
         |                      ~~~~^~~~~~
         |                          |
         |                          struct file *
   In file included from drivers/scsi/iscsi_tcp.c:25:
   include/linux/file.h:48:39: note: expected 'unsigned int' but argument is of type 'struct file *'
      48 | extern struct file *fget(unsigned int fd);
         |                          ~~~~~~~~~~~~~^~


vim +/fget +798 drivers/scsi/iscsi_tcp.c

   777	
   778	static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
   779					       enum iscsi_param param, char *buf)
   780	{
   781		struct iscsi_conn *conn = cls_conn->dd_data;
   782		struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
   783		struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
   784		struct sockaddr_in6 addr;
   785		struct socket *sock;
   786		int rc;
   787	
   788		switch(param) {
   789		case ISCSI_PARAM_CONN_PORT:
   790		case ISCSI_PARAM_CONN_ADDRESS:
   791		case ISCSI_PARAM_LOCAL_PORT:
   792			spin_lock_bh(&conn->session->frwd_lock);
   793			if (!tcp_sw_conn || !tcp_sw_conn->sock) {
   794				spin_unlock_bh(&conn->session->frwd_lock);
   795				return -ENOTCONN;
   796			}
   797			sock = tcp_sw_conn->sock;
 > 798			fget(sock->file);
   799			spin_unlock_bh(&conn->session->frwd_lock);
   800	
   801			if (param == ISCSI_PARAM_LOCAL_PORT)
   802				rc = kernel_getsockname(sock,
   803							(struct sockaddr *)&addr);
   804			else
   805				rc = kernel_getpeername(sock,
   806							(struct sockaddr *)&addr);
   807			spin_lock_bh(&conn->session->frwd_lock);
   808			sockfd_put(sock);
   809			spin_unlock_bh(&conn->session->frwd_lock);
   810			if (rc < 0)
   811				return rc;
   812	
   813			return iscsi_conn_get_addr_param((struct sockaddr_storage *)
   814							 &addr, param, buf);
   815		default:
   816			return iscsi_conn_get_param(cls_conn, param, buf);
   817		}
   818	
   819		return 0;
   820	}
   821	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[kernel test robot]

Hi Li,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mkp-scsi/for-next]
[also build test ERROR on jejb-scsi/for-next linus/master v5.19 next-20220805]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-r033-20220801 (https://download.01.org/0day-ci/archive/20220808/202208080020.xQk6IIBw-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/ccc367df3fdba07b24eeda721ca928cce50f40d2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
        git checkout ccc367df3fdba07b24eeda721ca928cce50f40d2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/scsi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/scsi/iscsi_tcp.c:798:8: error: incompatible pointer to integer conversion passing 'struct file *' to parameter of type 'unsigned int' [-Wint-conversion]
                   fget(sock->file);
                        ^~~~~~~~~~
   include/linux/file.h:48:39: note: passing argument to parameter 'fd' here
   extern struct file *fget(unsigned int fd);
                                         ^
   drivers/scsi/iscsi_tcp.c:852:8: error: incompatible pointer to integer conversion passing 'struct file *' to parameter of type 'unsigned int' [-Wint-conversion]
                   fget(sock->file);
                        ^~~~~~~~~~
   include/linux/file.h:48:39: note: passing argument to parameter 'fd' here
   extern struct file *fget(unsigned int fd);
                                         ^
   2 errors generated.


vim +798 drivers/scsi/iscsi_tcp.c

   777	
   778	static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
   779					       enum iscsi_param param, char *buf)
   780	{
   781		struct iscsi_conn *conn = cls_conn->dd_data;
   782		struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
   783		struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
   784		struct sockaddr_in6 addr;
   785		struct socket *sock;
   786		int rc;
   787	
   788		switch(param) {
   789		case ISCSI_PARAM_CONN_PORT:
   790		case ISCSI_PARAM_CONN_ADDRESS:
   791		case ISCSI_PARAM_LOCAL_PORT:
   792			spin_lock_bh(&conn->session->frwd_lock);
   793			if (!tcp_sw_conn || !tcp_sw_conn->sock) {
   794				spin_unlock_bh(&conn->session->frwd_lock);
   795				return -ENOTCONN;
   796			}
   797			sock = tcp_sw_conn->sock;
 > 798			fget(sock->file);
   799			spin_unlock_bh(&conn->session->frwd_lock);
   800	
   801			if (param == ISCSI_PARAM_LOCAL_PORT)
   802				rc = kernel_getsockname(sock,
   803							(struct sockaddr *)&addr);
   804			else
   805				rc = kernel_getpeername(sock,
   806							(struct sockaddr *)&addr);
   807			spin_lock_bh(&conn->session->frwd_lock);
   808			sockfd_put(sock);
   809			spin_unlock_bh(&conn->session->frwd_lock);
   810			if (rc < 0)
   811				return rc;
   812	
   813			return iscsi_conn_get_addr_param((struct sockaddr_storage *)
   814							 &addr, param, buf);
   815		default:
   816			return iscsi_conn_get_param(cls_conn, param, buf);
   817		}
   818	
   819		return 0;
   820	}
   821	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] scsi: iscsi: iscsi_tcp: Fix null-ptr-deref while calling getpeername()

[kernel test robot]

Hi Li,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on jejb-scsi/for-next linus/master v5.19]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: powerpc-randconfig-s051-20220801 (https://download.01.org/0day-ci/archive/20220808/202208081109.mO6WgY4E-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-39-gce1a6720-dirty
        # https://github.com/intel-lab-lkp/linux/commit/ccc367df3fdba07b24eeda721ca928cce50f40d2
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Li-Jinlin/scsi-iscsi-iscsi_tcp-Fix-null-ptr-deref-while-calling-getpeername/20220802-173945
        git checkout ccc367df3fdba07b24eeda721ca928cce50f40d2
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/scsi/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/iscsi_tcp.c:798:26: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int fd @@     got struct file *file @@
   drivers/scsi/iscsi_tcp.c:798:26: sparse:     expected unsigned int fd
   drivers/scsi/iscsi_tcp.c:798:26: sparse:     got struct file *file
   drivers/scsi/iscsi_tcp.c:852:26: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected unsigned int fd @@     got struct file *file @@
   drivers/scsi/iscsi_tcp.c:852:26: sparse:     expected unsigned int fd
   drivers/scsi/iscsi_tcp.c:852:26: sparse:     got struct file *file

vim +798 drivers/scsi/iscsi_tcp.c

   777	
   778	static int iscsi_sw_tcp_conn_get_param(struct iscsi_cls_conn *cls_conn,
   779					       enum iscsi_param param, char *buf)
   780	{
   781		struct iscsi_conn *conn = cls_conn->dd_data;
   782		struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
   783		struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
   784		struct sockaddr_in6 addr;
   785		struct socket *sock;
   786		int rc;
   787	
   788		switch(param) {
   789		case ISCSI_PARAM_CONN_PORT:
   790		case ISCSI_PARAM_CONN_ADDRESS:
   791		case ISCSI_PARAM_LOCAL_PORT:
   792			spin_lock_bh(&conn->session->frwd_lock);
   793			if (!tcp_sw_conn || !tcp_sw_conn->sock) {
   794				spin_unlock_bh(&conn->session->frwd_lock);
   795				return -ENOTCONN;
   796			}
   797			sock = tcp_sw_conn->sock;
 > 798			fget(sock->file);
   799			spin_unlock_bh(&conn->session->frwd_lock);
   800	
   801			if (param == ISCSI_PARAM_LOCAL_PORT)
   802				rc = kernel_getsockname(sock,
   803							(struct sockaddr *)&addr);
   804			else
   805				rc = kernel_getpeername(sock,
   806							(struct sockaddr *)&addr);
   807			spin_lock_bh(&conn->session->frwd_lock);
   808			sockfd_put(sock);
   809			spin_unlock_bh(&conn->session->frwd_lock);
   810			if (rc < 0)
   811				return rc;
   812	
   813			return iscsi_conn_get_addr_param((struct sockaddr_storage *)
   814							 &addr, param, buf);
   815		default:
   816			return iscsi_conn_get_param(cls_conn, param, buf);
   817		}
   818	
   819		return 0;
   820	}
   821	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp