On 9/27/20 11:33 PM, Mark Mielke wrote:
> Kernel may fail to boot or devices may fail to come up when
> initializing iscsi_tcp devices starting with Linux 5.8.
> 
> Marc Dionne identified the cause in RHBZ#1877345.
> 
> Commit a79af8a64d39 ("[SCSI] iscsi_tcp: use iscsi_conn_get_addr_param
> libiscsi function") introduced getpeername() within the session spinlock.
> 
> Commit 1b66d253610c ("bpf: Add get{peer, sock}name attach types for
> sock_addr") introduced BPF_CGROUP_RUN_SA_PROG_LOCK() within getpeername,
> which acquires a mutex and when used from iscsi_tcp devices can now lead
> to "BUG: scheduling while atomic:" and subsequent damage.
> 
> This commit ensures that the spinlock is released before calling
> getpeername() or getsockname(). sock_hold() and sock_put() are
> used to ensure that the socket reference is preserved until after
> the getpeername() or getsockname() complete.
> 
> Reported-by: Marc Dionne <marc.c.dio...@gmail.com>
> Link: 
> https://urldefense.com/v3/__https://bugzilla.redhat.com/show_bug.cgi?id=1877345__;!!GqivPVa7Brio!IykqCqCEtE_EyrhXerYzj_cIlmkenkaAVddyoEOw9T6n4nExSaGFHkn0ZQrLBVSUtxQ_$
>  
> Link: 
> https://urldefense.com/v3/__https://lkml.org/lkml/2020/7/28/1085__;!!GqivPVa7Brio!IykqCqCEtE_EyrhXerYzj_cIlmkenkaAVddyoEOw9T6n4nExSaGFHkn0ZQrLBRT9NL69$
>  
> Link: 
> https://urldefense.com/v3/__https://lkml.org/lkml/2020/8/31/459__;!!GqivPVa7Brio!IykqCqCEtE_EyrhXerYzj_cIlmkenkaAVddyoEOw9T6n4nExSaGFHkn0ZQrLBfxZYLKs$
>  
> Fixes: a79af8a64d39 ("[SCSI] iscsi_tcp: use iscsi_conn_get_addr_param 
> libiscsi function")
> Fixes: 1b66d253610c ("bpf: Add get{peer, sock}name attach types for 
> sock_addr")
> Signed-off-by: Mark Mielke <mark.mie...@gmail.com>
> Cc: sta...@vger.kernel.org
> ---
>  drivers/scsi/iscsi_tcp.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
> index b5dd1caae5e9..d10efb66cf19 100644
> --- a/drivers/scsi/iscsi_tcp.c
> +++ b/drivers/scsi/iscsi_tcp.c
> @@ -736,6 +736,7 @@ static int iscsi_sw_tcp_conn_get_param(struct 
> iscsi_cls_conn *cls_conn,
>       struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
>       struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
>       struct sockaddr_in6 addr;
> +     struct socket *sock;
>       int rc;
>  
>       switch(param) {
> @@ -747,13 +748,17 @@ static int iscsi_sw_tcp_conn_get_param(struct 
> iscsi_cls_conn *cls_conn,
>                       spin_unlock_bh(&conn->session->frwd_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(tcp_sw_conn->sock,
> +                     rc = kernel_getsockname(sock,
>                                               (struct sockaddr *)&addr);
>               else
> -                     rc = kernel_getpeername(tcp_sw_conn->sock,
> +                     rc = kernel_getpeername(sock,
>                                               (struct sockaddr *)&addr);
> -             spin_unlock_bh(&conn->session->frwd_lock);
> +             sock_put(sock->sk);
>               if (rc < 0)
>                       return rc;
>  
> @@ -775,6 +780,7 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host 
> *shost,
>       struct iscsi_tcp_conn *tcp_conn;
>       struct iscsi_sw_tcp_conn *tcp_sw_conn;
>       struct sockaddr_in6 addr;
> +     struct socket *sock;
>       int rc;
>  
>       switch (param) {
> @@ -789,16 +795,18 @@ static int iscsi_sw_tcp_host_get_param(struct Scsi_Host 
> *shost,
>                       return -ENOTCONN;
>               }
>               tcp_conn = conn->dd_data;
> -
>               tcp_sw_conn = tcp_conn->dd_data;
> -             if (!tcp_sw_conn->sock) {
> +             sock = tcp_sw_conn->sock;
> +             if (!sock) {
>                       spin_unlock_bh(&session->frwd_lock);
>                       return -ENOTCONN;
>               }
> +             sock_hold(sock->sk);
> +             spin_unlock_bh(&session->frwd_lock);
>  
> -             rc = kernel_getsockname(tcp_sw_conn->sock,
> +             rc = kernel_getsockname(sock,
>                                       (struct sockaddr *)&addr);
> -             spin_unlock_bh(&session->frwd_lock);
> +             sock_put(sock->sk);
>               if (rc < 0)
>                       return rc;
>  
> 

Reviewed-by: Mike Christie <michael.chris...@oracle.com>

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/aec41066-f5d3-c426-11c1-25e9b0a9ed44%40oracle.com.

Reply via email to