lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 14 Mar 2023 17:27:19 +0100
From:   Hannes Reinecke <hare@...e.de>
To:     Lee Duncan <leeman.duncan@...il.com>, linux-scsi@...r.kernel.org,
        open-iscsi@...glegroups.com, netdev@...r.kernel.org
Cc:     Lee Duncan <lduncan@...e.com>, Chris Leech <cleech@...hat.com>
Subject: Re: [RFC PATCH 4/9] iscsi: make all iSCSI netlink multicast namespace
 aware

On 2/8/23 18:40, Lee Duncan wrote:
> From: Lee Duncan <lduncan@...e.com>
> 
> Make use of the per-net netlink sockets. Responses are sent back on the
> same socket/namespace the request was received on.  Async events are
> reported on the socket/namespace stored in the iscsi_cls_host associated
> with the event.
> 
> Signed-off-by: Chris Leech <cleech@...hat.com>
> Signed-off-by: Lee Duncan <lduncan@...e.com>
> ---
>   drivers/scsi/scsi_transport_iscsi.c | 92 +++++++++++++++++++----------
>   1 file changed, 60 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index 2e2b291bce41..230b43d34c5f 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -2653,8 +2653,8 @@ iscsi_if_transport_lookup(struct iscsi_transport *tt)
>   }
>   
>   static int
> -iscsi_multicast_netns(struct net *net, struct sk_buff *skb,
> -		      uint32_t group, gfp_t gfp)
> +iscsi_multicast_skb(struct net *net, struct sk_buff *skb,
> +		    uint32_t group, gfp_t gfp)
>   {
>   	struct sock *nls;
>   	struct iscsi_net *isn;
> @@ -2665,17 +2665,10 @@ iscsi_multicast_netns(struct net *net, struct sk_buff *skb,
>   }
>   
>   static int
> -iscsi_multicast_skb(struct sk_buff *skb, uint32_t group, gfp_t gfp)
> -{
> -	return iscsi_multicast_netns(&init_net, skb, group, gfp);
> -}
> -
> -static int
> -iscsi_unicast_skb(struct sk_buff *skb, u32 portid)
> +iscsi_unicast_skb(struct net *net, struct sk_buff *skb, u32 portid)
>   {
>   	struct sock *nls;
>   	struct iscsi_net *isn;
> -	struct net *net = &init_net;
>   
>   	isn = net_generic(net, iscsi_net_id);
>   	nls = isn->nls;
> @@ -2690,6 +2683,7 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct iscsi_hdr *hdr,
>   	struct iscsi_uevent *ev;
>   	char *pdu;
>   	struct iscsi_internal *priv;
> +	struct net *net;
>   	int len = nlmsg_total_size(sizeof(*ev) + sizeof(struct iscsi_hdr) +
>   				   data_size);
>   
> @@ -2716,7 +2710,8 @@ int iscsi_recv_pdu(struct iscsi_cls_conn *conn, struct iscsi_hdr *hdr,
>   	memcpy(pdu, hdr, sizeof(struct iscsi_hdr));
>   	memcpy(pdu + sizeof(struct iscsi_hdr), data, data_size);
>   
> -	return iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
> +	net = iscsi_conn_net(conn);
> +	return iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
>   }
>   EXPORT_SYMBOL_GPL(iscsi_recv_pdu);
>   
> @@ -2727,6 +2722,7 @@ int iscsi_offload_mesg(struct Scsi_Host *shost,
>   	struct nlmsghdr	*nlh;
>   	struct sk_buff *skb;
>   	struct iscsi_uevent *ev;
> +	struct net *net;
>   	int len = nlmsg_total_size(sizeof(*ev) + data_size);
>   
>   	skb = alloc_skb(len, GFP_ATOMIC);
> @@ -2751,7 +2747,8 @@ int iscsi_offload_mesg(struct Scsi_Host *shost,
>   
>   	memcpy((char *)ev + sizeof(*ev), data, data_size);
>   
> -	return iscsi_multicast_skb(skb, ISCSI_NL_GRP_UIP, GFP_ATOMIC);
> +	net = iscsi_host_net(shost->shost_data);
> +	return iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_UIP, GFP_ATOMIC);
>   }
>   EXPORT_SYMBOL_GPL(iscsi_offload_mesg);
>   
> @@ -2761,6 +2758,7 @@ void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err error)
>   	struct sk_buff	*skb;
>   	struct iscsi_uevent *ev;
>   	struct iscsi_internal *priv;
> +	struct net *net;
>   	int len = nlmsg_total_size(sizeof(*ev));
>   	unsigned long flags;
>   	int state;
> @@ -2808,7 +2806,8 @@ void iscsi_conn_error_event(struct iscsi_cls_conn *conn, enum iscsi_err error)
>   	ev->r.connerror.cid = conn->cid;
>   	ev->r.connerror.sid = iscsi_conn_get_sid(conn);
>   
> -	iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
> +	net = iscsi_conn_net(conn);
> +	iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
>   
>   	iscsi_cls_conn_printk(KERN_INFO, conn, "detected conn error (%d)\n",
>   			      error);
> @@ -2822,6 +2821,7 @@ void iscsi_conn_login_event(struct iscsi_cls_conn *conn,
>   	struct sk_buff  *skb;
>   	struct iscsi_uevent *ev;
>   	struct iscsi_internal *priv;
> +	struct net *net;
>   	int len = nlmsg_total_size(sizeof(*ev));
>   
>   	priv = iscsi_if_transport_lookup(conn->transport);
> @@ -2842,7 +2842,9 @@ void iscsi_conn_login_event(struct iscsi_cls_conn *conn,
>   	ev->r.conn_login.state = state;
>   	ev->r.conn_login.cid = conn->cid;
>   	ev->r.conn_login.sid = iscsi_conn_get_sid(conn);
> -	iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
> +
> +	net = iscsi_conn_net(conn);
> +	iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_ATOMIC);
>   
>   	iscsi_cls_conn_printk(KERN_INFO, conn, "detected conn login (%d)\n",
>   			      state);
> @@ -2853,11 +2855,17 @@ void iscsi_post_host_event(uint32_t host_no, struct iscsi_transport *transport,
>   			   enum iscsi_host_event_code code, uint32_t data_size,
>   			   uint8_t *data)
>   {
> +	struct Scsi_Host *shost;
> +	struct net *net;
>   	struct nlmsghdr *nlh;
>   	struct sk_buff *skb;
>   	struct iscsi_uevent *ev;
>   	int len = nlmsg_total_size(sizeof(*ev) + data_size);
>   
> +	shost = scsi_host_lookup(host_no);
> +	if (!shost)
> +		return;
> +
>   	skb = alloc_skb(len, GFP_NOIO);
>   	if (!skb) {
>   		printk(KERN_ERR "gracefully ignored host event (%d):%d OOM\n",
> @@ -2876,7 +2884,9 @@ void iscsi_post_host_event(uint32_t host_no, struct iscsi_transport *transport,
>   	if (data_size)
>   		memcpy((char *)ev + sizeof(*ev), data, data_size);
>   
> -	iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
> +	net = iscsi_host_net(shost->shost_data);
> +	scsi_host_put(shost);
> +	iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
>   }
>   EXPORT_SYMBOL_GPL(iscsi_post_host_event);
>   
> @@ -2884,11 +2894,17 @@ void iscsi_ping_comp_event(uint32_t host_no, struct iscsi_transport *transport,
>   			   uint32_t status, uint32_t pid, uint32_t data_size,
>   			   uint8_t *data)
>   {
> +	struct Scsi_Host *shost;
> +	struct net *net;
>   	struct nlmsghdr *nlh;
>   	struct sk_buff *skb;
>   	struct iscsi_uevent *ev;
>   	int len = nlmsg_total_size(sizeof(*ev) + data_size);
>   
> +	shost = scsi_host_lookup(host_no);
> +	if (!shost)
> +		return;
> +
>   	skb = alloc_skb(len, GFP_NOIO);
>   	if (!skb) {
>   		printk(KERN_ERR "gracefully ignored ping comp: OOM\n");
> @@ -2905,12 +2921,15 @@ void iscsi_ping_comp_event(uint32_t host_no, struct iscsi_transport *transport,
>   	ev->r.ping_comp.data_size = data_size;
>   	memcpy((char *)ev + sizeof(*ev), data, data_size);
>   
> -	iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
> +	net = iscsi_host_net(shost->shost_data);
> +	scsi_host_put(shost);
> +	iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
>   }
>   EXPORT_SYMBOL_GPL(iscsi_ping_comp_event);
>   
>   static int
> -iscsi_if_send_reply(u32 portid, int type, void *payload, int size)
> +iscsi_if_send_reply(struct net *net, u32 portid, int type,
> +		    void *payload, int size)
>   {
>   	struct sk_buff	*skb;
>   	struct nlmsghdr	*nlh;
> @@ -2924,11 +2943,11 @@ iscsi_if_send_reply(u32 portid, int type, void *payload, int size)
>   
>   	nlh = __nlmsg_put(skb, 0, 0, type, (len - sizeof(*nlh)), 0);
>   	memcpy(nlmsg_data(nlh), payload, size);
> -	return iscsi_unicast_skb(skb, portid);
> +	return iscsi_unicast_skb(net, skb, portid);
>   }
>   
>   static int
> -iscsi_if_get_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
> +iscsi_if_get_stats(struct net *net, struct iscsi_transport *transport, struct nlmsghdr *nlh)
>   {
>   	struct iscsi_uevent *ev = nlmsg_data(nlh);
>   	struct iscsi_stats *stats;
> @@ -2985,7 +3004,7 @@ iscsi_if_get_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
>   		skb_trim(skbstat, NLMSG_ALIGN(actual_size));
>   		nlhstat->nlmsg_len = actual_size;
>   
> -		err = iscsi_multicast_skb(skbstat, ISCSI_NL_GRP_ISCSID,
> +		err = iscsi_multicast_skb(net, skbstat, ISCSI_NL_GRP_ISCSID,
>   					  GFP_ATOMIC);
>   	} while (err < 0 && err != -ECONNREFUSED);
>   
> @@ -3005,6 +3024,7 @@ int iscsi_session_event(struct iscsi_cls_session *session,
>   	struct iscsi_uevent *ev;
>   	struct sk_buff  *skb;
>   	struct nlmsghdr *nlh;
> +	struct net *net;
>   	int rc, len = nlmsg_total_size(sizeof(*ev));
>   
>   	priv = iscsi_if_transport_lookup(session->transport);
> @@ -3049,7 +3069,8 @@ int iscsi_session_event(struct iscsi_cls_session *session,
>   	 * this will occur if the daemon is not up, so we just warn
>   	 * the user and when the daemon is restarted it will handle it
>   	 */
> -	rc = iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_KERNEL);
> +	net = iscsi_sess_net(session);
> +	rc = iscsi_multicast_skb(net, skb, ISCSI_NL_GRP_ISCSID, GFP_KERNEL);
>   	if (rc == -ESRCH)
>   		iscsi_cls_session_printk(KERN_ERR, session,
>   					 "Cannot notify userspace of session "
> @@ -3412,7 +3433,8 @@ iscsi_send_ping(struct iscsi_transport *transport, struct iscsi_uevent *ev)
>   }
>   
>   static int
> -iscsi_get_chap(struct iscsi_transport *transport, struct nlmsghdr *nlh)
> +iscsi_get_chap(struct net *net, struct iscsi_transport *transport,
> +	       struct nlmsghdr *nlh)
>   {
>   	struct iscsi_uevent *ev = nlmsg_data(nlh);
>   	struct Scsi_Host *shost = NULL;
> @@ -3471,7 +3493,7 @@ iscsi_get_chap(struct iscsi_transport *transport, struct nlmsghdr *nlh)
>   		skb_trim(skbchap, NLMSG_ALIGN(actual_size));
>   		nlhchap->nlmsg_len = actual_size;
>   
> -		err = iscsi_multicast_skb(skbchap, ISCSI_NL_GRP_ISCSID,
> +		err = iscsi_multicast_skb(net, skbchap, ISCSI_NL_GRP_ISCSID,
>   					  GFP_KERNEL);
>   	} while (err < 0 && err != -ECONNREFUSED);
>   
> @@ -3818,7 +3840,8 @@ static int iscsi_logout_flashnode_sid(struct iscsi_transport *transport,
>   }
>   
>   static int
> -iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
> +iscsi_get_host_stats(struct net *net, struct iscsi_transport *transport,
> +		     struct nlmsghdr *nlh)
>   {
>   	struct iscsi_uevent *ev = nlmsg_data(nlh);
>   	struct Scsi_Host *shost = NULL;
> @@ -3878,8 +3901,8 @@ iscsi_get_host_stats(struct iscsi_transport *transport, struct nlmsghdr *nlh)
>   		skb_trim(skbhost_stats, NLMSG_ALIGN(actual_size));
>   		nlhhost_stats->nlmsg_len = actual_size;
>   
> -		err = iscsi_multicast_skb(skbhost_stats, ISCSI_NL_GRP_ISCSID,
> -					  GFP_KERNEL);
> +		err = iscsi_multicast_skb(net, skbhost_stats,
> +					  ISCSI_NL_GRP_ISCSID, GFP_KERNEL);
>   	} while (err < 0 && err != -ECONNREFUSED);
>   
>   exit_host_stats:
> @@ -4001,7 +4024,8 @@ static int iscsi_if_transport_conn(struct iscsi_transport *transport,
>   }
>   
>   static int
> -iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
> +iscsi_if_recv_msg(struct net *net, struct sk_buff *skb,
> +		  struct nlmsghdr *nlh, uint32_t *group)
>   {
>   	int err = 0;
>   	u32 portid;
> @@ -4096,7 +4120,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
>   		err = iscsi_if_transport_conn(transport, nlh);
>   		break;
>   	case ISCSI_UEVENT_GET_STATS:
> -		err = iscsi_if_get_stats(transport, nlh);
> +		err = iscsi_if_get_stats(net, transport, nlh);
>   		break;
>   	case ISCSI_UEVENT_TRANSPORT_EP_CONNECT:
>   	case ISCSI_UEVENT_TRANSPORT_EP_POLL:
> @@ -4121,7 +4145,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
>   		err = iscsi_send_ping(transport, ev);
>   		break;
>   	case ISCSI_UEVENT_GET_CHAP:
> -		err = iscsi_get_chap(transport, nlh);
> +		err = iscsi_get_chap(net, transport, nlh);
>   		break;
>   	case ISCSI_UEVENT_DELETE_CHAP:
>   		err = iscsi_delete_chap(transport, ev);
> @@ -4152,7 +4176,7 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
>   				     nlmsg_attrlen(nlh, sizeof(*ev)));
>   		break;
>   	case ISCSI_UEVENT_GET_HOST_STATS:
> -		err = iscsi_get_host_stats(transport, nlh);
> +		err = iscsi_get_host_stats(net, transport, nlh);
>   		break;
>   	default:
>   		err = -ENOSYS;
> @@ -4170,6 +4194,8 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
>   static void
>   iscsi_if_rx(struct sk_buff *skb)
>   {
> +	struct sock *sk = skb->sk;
> +	struct net *net = sock_net(sk);
>   	u32 portid = NETLINK_CB(skb).portid;
>   
>   	mutex_lock(&rx_queue_mutex);
> @@ -4192,7 +4218,7 @@ iscsi_if_rx(struct sk_buff *skb)
>   		if (rlen > skb->len)
>   			rlen = skb->len;
>   
> -		err = iscsi_if_recv_msg(skb, nlh, &group);
> +		err = iscsi_if_recv_msg(net, skb, nlh, &group);
>   		if (err) {
>   			ev->type = ISCSI_KEVENT_IF_ERROR;
>   			ev->iferror = err;
> @@ -4208,7 +4234,9 @@ iscsi_if_rx(struct sk_buff *skb)
>   				break;
>   			if (ev->type == ISCSI_UEVENT_GET_CHAP && !err)
>   				break;
> -			err = iscsi_if_send_reply(portid, nlh->nlmsg_type,
> +			if (ev->type == ISCSI_UEVENT_GET_HOST_STATS && !err)
> +				break;
> +			err = iscsi_if_send_reply(net, portid, nlh->nlmsg_type,
>   						  ev, sizeof(*ev));
>   			if (err == -EAGAIN && --retries < 0) {
>   				printk(KERN_WARNING "Send reply failed, error %d\n", err);

As discussed with Lee: you should tear down sessions related to this 
namespace from the pernet ->exit callback, otherwise you end up with 
session which can no longer been reached as the netlink socket is gone.

Might be done in an additional patch, though.
If you do you can add:

Reviewed-by: Hannes Reinecke <hare@...e.de>

to this patch.

Cheers,

Hannes

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ