Re: [PATCH net v15] net/smc: fix lgr/lnk lifetime vs diag reader race

[Sidraya Jayagond <sidraya@linux.ibm.com>]

On 02/11/26 2:01 pm, Mahanta Jambigi wrote:
> The diag dump walks the socket hash table under a read_lock and
> dereferences conn->lgr and conn->lnk. Several paths expose a window
> where a hashed socket has freed or partially freed conn/lgr/lnk state.
> 
> First, smc_conn_abort() calls smc_conn_free() during early handshake
> aborts while the socket is still hashed in SMC_INIT, synchronously
> freeing the lgr on the smc_lgr_cleanup_early() path without holding the
> hash write_lock. A concurrent diag reader can load a non-NULL conn->lgr
> and dereference it after the kfree. Skip the connection, link group and
> DMB attributes in __smc_diag_dump() for sockets in SMC_INIT or
> SMC_CLOSED or fallen back to TCP, and take diag_mode from an is_smcd
> flag cached at connection creation.
> 
> The state filter alone does not close the race. A connection aborted by
> a failed handshake can keep its alert token, because smc_conn_free()
> skips the unregister when smc_lgr_terminate_sched() has emptied
> lgr->list; close_work may also have been queued before the abort, as the
> CDC receive path is not excluded by the handshake's socket lock.
> smc_close_passive_work() would then move the hashed socket from SMC_INIT
> to SMC_APPCLOSEWAIT1 or SMC_PROCESSABORT, states the filter does not
> cover, and later run a passive-closing sock_put() that
> smc_connect_decline_fallback() dropped already. Return early from
> smc_close_passive_work() when the connection was freed; the check holds
> the socket lock that every smc_conn_free() caller holds as well, so a
> hashed socket with a freed connection stays in SMC_INIT or SMC_CLOSED
> until release.
> 
> This changes the sock_diag output for such sockets: the connection, link
> group and DMB attributes are omitted, which smcss does not read for
> sockets in these states, while SMC_DIAG_FALLBACK is still reported and
> an SMC-D socket in SMC_INIT or SMC_CLOSED still reports
> SMC_DIAG_MODE_SMCD from the cached flag. A fallback socket is a plain
> TCP socket with no SMC connection state to report; the stale conn fields
> left behind by smc_conn_abort() must not be read.
> 
> Second, in smc_conn_kill() on SMC-D with dmb_nocopy, smcd_buf_detach()
> NULLs and frees conn->sndbuf_desc while the socket is still hashed. A
> concurrent diag reader entering the CONNINFO block can dereference the
> freed descriptor. Unhash the socket right before smcd_buf_detach(). As a
> result, killed ACTIVE and APPCLOSEWAIT sockets that are still open are
> no longer visible to smcss on loopback-ism devices.
> 
> Third, in the terminal teardown branches of smc_close_active_abort()
> (PEERCLOSEWAIT and PROCESSABORT groups) and in smc_close_passive_work(),
> smc_conn_free() drops lgr and lnk references while the socket is still
> hashed. Unhash immediately before each smc_conn_free() call at those
> sites. Sockets killed in those close states are therefore unhashed, and
> their inuse accounting dropped, before close(), even if the fd is still
> open.
> 
> Fourth, use smp_load_acquire() / smp_store_release() on sk_state to
> order these checks on weakly ordered CPUs. A reader that observes
> SMC_ACTIVE is guaranteed to see the connection setup that preceded the
> store; a reader that observes SMC_CLOSED from smc_listen_out_err() skips
> the connection fields in any case, so that particular store is hardening
> kept for uniformity, not a required ordering.
> 
> Additionally, in __smc_diag_dump(), use the cached link->ibname instead
> of link->smcibdev->ibdev->name, avoiding a NULL pointer dereference or
> use-after-free of the IB device when __smcr_link_clear() zeroes the link
> during concurrent SMC-R link failover. The link data is still read
> without conns_lock or send_lock, so a dump racing a failover may
> transiently report zeroed or mixed link fields; this does not affect
> memory safety and the next dump reports the new link. Like the SMC
> netlink link dump and the tracepoints, the name is the one cached at
> link creation time.
> 
> Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
> Fixes: 2a0674fffb6b ("net/smc: improve abnormal termination of link groups")
> Fixes: 46c28dbd4c23 ("net/smc: no socket state changes in tasklet context")
> Fixes: 9dbe086c69b8 ("net/smc: fix invalid link access in dumping SMC-R connections")
> Fixes: ae2be35cbed2 ("net/smc: {at|de}tach sndbuf to peer DMB if supported")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
> ---
> Changes in v15:
> - smc_close.c: return early from smc_close_passive_work() when
>   conn->freed is set; the check runs under lock_sock and every
>   smc_conn_free() caller holds the socket lock, so it cannot race with
>   the free. A connection whose unregister was skipped because
>   smc_lgr_terminate_sched() had emptied lgr->list keeps its alert token
>   after a failed handshake, and a late peer CDC close or abort
>   (smc_cdc_rx_handler() does not check conn->freed) queues close_work,
>   which runs once the handshake thread has dropped the socket lock; it
>   could move the hashed socket from SMC_INIT to SMC_APPCLOSEWAIT1 or
>   SMC_PROCESSABORT, states the SMC_INIT/SMC_CLOSED filter in
>   __smc_diag_dump() does not cover, with conn->lgr pointing at a link
>   group whose last reference smc_conn_free() already dropped, and later
>   run a passive-closing sock_put() although
>   smc_connect_decline_fallback() already dropped that reference on the
>   failed handshake; add the Fixes tag for 2a0674fffb6b, which
>   introduced the unregister skip in smc_conn_free(); close_work is also
>   reachable without a stale token, because the CDC receive path queues
>   it under bh_lock_sock(), which does not exclude the handshake
>   thread's lock_sock(), so a close_work queued before the abort runs on
>   the freed connection as well; that variant exists since close_work
>   was introduced, hence the additional Fixes tag for 46c28dbd4c23
> - smc.h, smc_core.c, smc_diag.c: cache the connection type in
>   conn->is_smcd at smc_conn_create() and take diag_mode from it instead
>   of dereferencing conn->lgr->is_smcd; v14 reported every non-fallback
>   socket in SMC_INIT or SMC_CLOSED as SMC_DIAG_MODE_SMCR, including
>   SMC-D connections, which puts them in the wrong smcss -D / -R group;
>   SMC-D sockets in SMC_INIT after smc_conn_create() and SMC-D sockets
>   moved to SMC_CLOSED by smc_close_passive_work() or
>   smc_close_passive_abort_received() while the fd is still open are
>   affected; the v14 condition on diag_state in the diag_mode test is
>   dropped together with the lgr deref
> - commit message: reword the fourth item; with the SMC_CLOSED check in
>   __smc_diag_dump() a reader that observes SMC_CLOSED never reads
>   alert_token_local or conn->lgr, whatever the state was before
>   smc_listen_out_err() stored it, so the smp_store_release() there is
>   hardening, not a required ordering, and stable backporters should not
>   treat it as a standalone fix
> 
> Changes in v14:
> - smc_diag.c: also exclude SMC_CLOSED before dereferencing conn->lgr to
>   determine diag_mode; a server socket that fails in
>   smc_listen_decline() with rc < 0 reaches SMC_CLOSED hashed, with a
>   stale alert_token_local left by smc_conn_free() when
>   smc_lgr_terminate_sched() had already emptied lgr->list, and the lgr
>   can be freed by terminate_work while a diag reader still holds the
>   hash read_lock
> - commit message: document the diag_mode change for SMC_CLOSED and the
>   limits of the smp_store_release() in smc_listen_out_err()
> 
> Changes in v13:
> - smc_diag.c: also skip the CONNINFO/LGRINFO/DMBINFO blocks for sockets
>   with use_fallback set; the SMC_DIAG_FALLBACK attribute is still
>   reported. After smc_lgr_terminate_sched() emptied lgr->list,
>   smc_conn_free() skips smc_lgr_unregister_conn(), so a socket that then
>   falls back and reaches SMC_ACTIVE can keep a stale alert_token_local
>   and a diag reader could dereference the freed sndbuf/rmb/lgr; a
>   fallback socket is a plain TCP socket with no SMC connection state to
>   report
> - smc_core.c: drop the v11 and v12 changes (unconditional
>   smc_lgr_unregister_conn() in smc_conn_free() and the
>   !lgr->terminating check in smc_lgr_schedule_free_work()); the v12
>   check was racy against __smc_lgr_terminate() and the changes moved the
>   patch away from the diag reader race, so smc_core.c is back to the v10
>   change only; the Fixes tag for 2a0674fffb6b added in v11 is dropped
>   with them
> - commit message: state that using the cached link->ibname avoids the
>   NULL pointer dereference and the IB device use-after-free, and that
>   link fields may be transiently zeroed or mixed in a dump racing with a
>   link failover, without affecting memory safety
> - commit message: drop the claim that other kill paths leave the socket
>   hashed; every smc_conn_kill() ends in smc_close_active_abort(), so
>   sockets killed in the PEERCLOSEWAIT1/2, PEERFINCLOSEWAIT, PROCESSABORT
>   and APPFINCLOSEWAIT states are unhashed before close() even if the fd
>   is still open, and the dmb_nocopy unhash hides ACTIVE/APPCLOSEWAIT
>   sockets on loopback-ism; smcss visibility thus depends on close state
>   and device type; this supersedes the v10 entry below
> 
> Changes in v12:
> - smc_core.c: guard smc_lgr_schedule_free_work() with !lgr->terminating
>   in addition to !lgr->freeing, preventing free_work from being re-armed
>   on a terminating LGR after smc_lgr_cleanup_early() has called
>   __smc_lgr_terminate() (which sets terminating=1 and cancels free_work
>   without setting freeing=1); without this a concurrent smc_conn_free()
>   decrementing conns_num to 0 would re-arm free_work, and the subsequent
>   kfree(lgr) in smc_lgr_free() would leave the armed work pointing at
>   freed memory
> 
> Changes in v11:
> - smc_core.c: unregister connection unconditionally in smc_conn_free()
>   rather than gating it on !list_empty(&lgr->list), ensuring
>   alert_token_local is always reset to 0
> - commit message: add Fixes tag for 2a0674fffb6b and clarify that
>   copying cached link->ibname eliminates the NULL dereference / UAF
>   during concurrent link clearing
> 
> Changes in v10:
> - smc_core.c: move the unhash out of the top of smc_conn_kill() into the
>   smc_ism_support_dmb_nocopy() branch, right before smcd_buf_detach(); the
>   other kill paths leave ACTIVE/APPCLOSEWAIT sockets open, so they must
>   stay visible to smcss and keep their inuse accounting
> - commit message: document the sock_diag output changes for sockets in
>   SMC_INIT and SMC_CLOSED (extension attributes omitted, diag_mode of
>   SMC-D in SMC_INIT) and that ibname is the name cached at link creation
> 
> Changes in v9:
> - af_smc.c: change plain store in smc_listen_out_err() to
>   smp_store_release(&newsmcsk->sk_state, SMC_CLOSED) to order the
>   preceding alert_token_local clear (done under write_lock_bh() inside
>   smc_conn_free()) before the sk_state write; pairs with the
>   smp_load_acquire() in __smc_diag_dump() to close a use-after-free on
>   weakly ordered CPUs where a diag reader could observe SMC_CLOSED while
>   conn->lgr still appeared live
> 
> Changes in v8:
> - smc_diag.c: use smp_load_acquire(&sk->sk_state) to prevent
>   speculative load reordering on weakly ordered CPUs (such as arm64)
>   during handshake abort fallback
> - af_smc.c: pair with smp_store_release(&sk->sk_state, SMC_ACTIVE)
>   when transitioning out of SMC_INIT
> - smc_diag.c: copy cached link->ibname instead of chasing
>   link->smcibdev->ibdev->name to prevent NULL pointer dereference / UAF
>   if __smcr_link_clear() zeroes the link during link failover
> 
> Changes in v7:
> - no code changes; commit message only
> - document the SMC_CLOSED leg of the early-exit guard and the rationale
>   that smcss already skips CLOSED sockets on the userspace side; add
>   a Fixes tag for ae2be35cbed2 for the smc_conn_kill() hunk since
>   smcd_buf_detach() was introduced by that commit
> 
> Changes in v6:
> - smc_diag.c: inline sk->sk_state != SMC_INIT into the else-if
>   condition so conn->lgr->is_smcd is never loaded for SMC_INIT sockets;
>   retain the SMC_INIT || SMC_CLOSED early-exit guard and out: label
>   from v5 to defensively skip the CONNINFO/LGRINFO/DMBINFO blocks for
>   sockets with no live connection
> - smc_close.c: remove the unconditional unhash from the top of
>   smc_close_active_abort(); place it scoped immediately before each of
>   the two smc_conn_free() calls in the PEERCLOSEWAIT and PROCESSABORT
>   branches, and before smc_conn_free() in smc_close_passive_work();
>   this avoids prematurely hiding live SMC_ACTIVE/APPCLOSEWAIT sockets
>   from smcss
> - smc_core.c: unhash at the top of smc_conn_kill(), before
>   smcd_buf_detach(), so the socket is off the hash table before
>   conn->sndbuf_desc is freed on the dmb_nocopy path
> 
> Changes in v5:
> - guard conn/lgr/lnk blocks in __smc_diag_dump() with an
>   (SMC_INIT || SMC_CLOSED) state check placed after the
>   SMC_DIAG_FALLBACK nla_put; SMC_INIT can coexist with fallback
>   (smc_sendmsg() MSG_FASTOPEN path), so the guard must not suppress
>   the fallback reason
> 
> Changes in v4:
> - dropped smc_conn_unhash() wrapper, conn->unhashed flag, and all
>   changes to af_smc.c, smc.h, smc_core.c and smc_core.h;
>   smc_unhash_sk() is already idempotent via sk_hashed(), so direct
>   calls at the two teardown sites in smc_close.c are sufficient
> - dropped the __smc_release() hunk: it needs no change since the
>   subsequent unhash there is already a safe no-op
> - fixed premature-unhash issue present in v3: smc_conn_free() must not
>   unhash because smc_conn_abort() calls it before
>   smc_switch_to_fallback() in both smc_listen_decline() and
>   smc_connect_rdma() error paths; unhashing there would make live
>   fallback sockets invisible to smcss
> - likewise, the ISM/RDMA retry loops call smc_conn_abort() on a failed
>   attempt and then smc_conn_create() on the next device; unhashing in
>   smc_conn_free() would permanently hide the established connection from
>   smc_diag since smc_conn_create() does not re-hash the socket
> 
> Changes in v3:
> - redesigned as a single patch; dropped the lgr_lnk_lock spinlock
>   approach and the 2-patch split
> 
> Changes in v2:
> - split into a 2-patch series; patch 1/2 adds per-connection
>   lgr_lnk_lock infrastructure to smc_core, patch 2/2 fixes the diag
>   dump path using it
> 
> 
>  net/smc/af_smc.c    | 10 +++++-----
>  net/smc/smc.h       |  1 +
>  net/smc/smc_close.c | 14 ++++++++++++++
>  net/smc/smc_core.c  |  8 +++++++-
>  net/smc/smc_diag.c  | 12 ++++++++----
>  5 files changed, 35 insertions(+), 10 deletions(-)
> 
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index d10244af5bd2..ec5980add7a9 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -980,7 +980,7 @@ static int smc_connect_fallback(struct smc_sock *smc, int reason_code)
>  	smc_copy_sock_settings_to_clc(smc);
>  	smc->connect_nonblock = 0;
>  	if (smc->sk.sk_state == SMC_INIT)
> -		smc->sk.sk_state = SMC_ACTIVE;
> +		smp_store_release(&smc->sk.sk_state, SMC_ACTIVE);
>  	return 0;
>  }
> 
> @@ -1385,7 +1385,7 @@ static int smc_connect_rdma(struct smc_sock *smc,
>  	smc_copy_sock_settings_to_clc(smc);
>  	smc->connect_nonblock = 0;
>  	if (smc->sk.sk_state == SMC_INIT)
> -		smc->sk.sk_state = SMC_ACTIVE;
> +		smp_store_release(&smc->sk.sk_state, SMC_ACTIVE);
> 
>  	return 0;
>  connect_abort:
> @@ -1488,7 +1488,7 @@ static int smc_connect_ism(struct smc_sock *smc,
>  	smc_copy_sock_settings_to_clc(smc);
>  	smc->connect_nonblock = 0;
>  	if (smc->sk.sk_state == SMC_INIT)
> -		smc->sk.sk_state = SMC_ACTIVE;
> +		smp_store_release(&smc->sk.sk_state, SMC_ACTIVE);
> 
>  	return 0;
>  connect_abort:
> @@ -1957,7 +1957,7 @@ static void smc_listen_out_connected(struct smc_sock *new_smc)
>  	struct sock *newsmcsk = &new_smc->sk;
> 
>  	if (newsmcsk->sk_state == SMC_INIT)
> -		newsmcsk->sk_state = SMC_ACTIVE;
> +		smp_store_release(&newsmcsk->sk_state, SMC_ACTIVE);
> 
>  	smc_listen_out(new_smc);
>  }
> @@ -1971,7 +1971,7 @@ static void smc_listen_out_err(struct smc_sock *new_smc)
>  	this_cpu_inc(net->smc.smc_stats->srv_hshake_err_cnt);
>  	if (newsmcsk->sk_state == SMC_INIT)
>  		sock_put(&new_smc->sk); /* passive closing */
> -	newsmcsk->sk_state = SMC_CLOSED;
> +	smp_store_release(&newsmcsk->sk_state, SMC_CLOSED);
> 
>  	smc_listen_out(new_smc);
>  }
> diff --git a/net/smc/smc.h b/net/smc/smc.h
> index 427b6d63b993..36b243b34a9a 100644
> --- a/net/smc/smc.h
> +++ b/net/smc/smc.h
> @@ -196,6 +196,7 @@ struct smc_connection {
>  	struct rb_node		alert_node;
>  	struct smc_link_group	*lgr;		/* link group of connection */
>  	struct smc_link		*lnk;		/* assigned SMC-R link */
> +	bool			is_smcd;	/* SMC-R or SMC-D connection */
>  	u32			alert_token_local; /* unique conn. id */
>  	u8			peer_rmbe_idx;	/* from tcp handshake */
>  	int			peer_rmbe_size;	/* size of peer rx buffer */
> diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
> index a8d4b4b65f24..dcb560f1947c 100644
> --- a/net/smc/smc_close.c
> +++ b/net/smc/smc_close.c
> @@ -155,6 +155,7 @@ void smc_close_active_abort(struct smc_sock *smc)
>  		if (sk->sk_state != SMC_PEERABORTWAIT)
>  			break;
>  		sk->sk_state = SMC_CLOSED;
> +		sk->sk_prot->unhash(sk);
>  		smc_conn_free(&smc->conn);
>  		release_clcsock = true;
>  		sock_put(sk); /* passive closing */
> @@ -166,6 +167,7 @@ void smc_close_active_abort(struct smc_sock *smc)
>  		if (sk->sk_state != SMC_PEERABORTWAIT)
>  			break;
>  		sk->sk_state = SMC_CLOSED;
> +		sk->sk_prot->unhash(sk);
>  		smc_conn_free(&smc->conn);
>  		release_clcsock = true;
>  		break;
> @@ -370,6 +372,17 @@ static void smc_close_passive_work(struct work_struct *work)
>  	lock_sock(sk);
>  	old_state = sk->sk_state;
> 
> +	if (conn->freed) {
> +		/* conn->freed is set under the socket lock, which is
> +		 * held here as well, so this cannot race with the free; the
> +		 * state machine and its postponed sock_put() are not owed
> +		 * anymore
> +		 */
> +		release_sock(sk);
> +		sock_put(sk); /* sock_hold done by schedulers of close_work */
> +		return;
> +	}
> +
>  	rxflags = &conn->local_rx_ctrl.conn_state_flags;
>  	if (rxflags->peer_conn_abort) {
>  		/* peer has not received all data */
> @@ -437,6 +449,7 @@ static void smc_close_passive_work(struct work_struct *work)
>  		sk->sk_state_change(sk);
>  		if ((sk->sk_state == SMC_CLOSED) &&
>  		    (sock_flag(sk, SOCK_DEAD) || !sk->sk_socket)) {
> +			sk->sk_prot->unhash(sk);
>  			smc_conn_free(conn);
>  			if (smc->clcsock)
>  				release_clcsock = true;
> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 6021322253e9..0a9f5418329a 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1532,8 +1532,10 @@ static void smc_conn_kill(struct smc_connection *conn, bool soft)
>  			tasklet_kill(&conn->rx_tsklet);
>  		else
>  			tasklet_unlock_wait(&conn->rx_tsklet);
> -		if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
> +		if (smc_ism_support_dmb_nocopy(conn->lgr->smcd)) {
> +			smc->sk.sk_prot->unhash(&smc->sk);
>  			smcd_buf_detach(conn);
> +		}
>  	} else {
>  		smc_cdc_wait_pend_tx_wr(conn);
>  	}
> @@ -2053,6 +2055,10 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini)
>  			goto out;
>  		}
>  	}
> +	/* cache the connection type: smc_diag reads it after
> +	 * smc_conn_free() dropped the link group reference
> +	 */
> +	conn->is_smcd = conn->lgr->is_smcd;

This could lead to stale value on failed retries. is_smcd is only
assigned on the success path of smc_conn_create(). Take a listen side
ISM v2 attempt that succeeds and then aborts, followed by an SMC-R
attempt whose smc_conn_create() fails before reaching this line. If the
socket then closes without falling back, it reports SMC_DIAG_MODE_SMCD
for an SMC-R attempt. This is cosmetic, but you could clear it at the
start of smc_conn_create().

>  	smc_lgr_hold(conn->lgr); /* lgr_put in smc_conn_free() */
>  	if (!conn->lgr->is_smcd)
>  		smcr_link_hold(conn->lnk); /* link_put in smc_conn_free() */
> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
> index bf0beaa23bdb..15485a08d9a0 100644
> --- a/net/smc/smc_diag.c
> +++ b/net/smc/smc_diag.c
> @@ -87,10 +87,10 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
> 
>  	r = nlmsg_data(nlh);
>  	smc_diag_msg_common_fill(r, sk);
> -	r->diag_state = sk->sk_state;
> +	r->diag_state = smp_load_acquire(&sk->sk_state);
>  	if (smc->use_fallback)
>  		r->diag_mode = SMC_DIAG_MODE_FALLBACK_TCP;
> -	else if (smc_conn_lgr_valid(&smc->conn) && smc->conn.lgr->is_smcd)
> +	else if (smc->conn.is_smcd)
>  		r->diag_mode = SMC_DIAG_MODE_SMCD;
>  	else
>  		r->diag_mode = SMC_DIAG_MODE_SMCR;
> @@ -103,6 +103,10 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
>  	if (nla_put(skb, SMC_DIAG_FALLBACK, sizeof(fallback), &fallback) < 0)
>  		goto errout;
> 
> +	if (r->diag_state == SMC_INIT || r->diag_state == SMC_CLOSED ||
> +	    smc->use_fallback)
> +		goto out;
> +
>  	if ((req->diag_ext & (1 << (SMC_DIAG_CONNINFO - 1))) &&
>  	    smc->conn.alert_token_local) {
>  		struct smc_connection *conn = &smc->conn;
> @@ -154,8 +158,7 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
>  			.lnk[0].link_id = link->link_id,
>  		};
> 
> -		memcpy(linfo.lnk[0].ibname, link->smcibdev->ibdev->name,
> -		       sizeof(link->smcibdev->ibdev->name));
> +		memcpy(linfo.lnk[0].ibname, link->ibname, sizeof(link->ibname));
>  		smc_gid_be16_convert(linfo.lnk[0].gid, link->gid);
>  		smc_gid_be16_convert(linfo.lnk[0].peer_gid, link->peer_gid);
> 
> @@ -185,6 +188,7 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
>  			goto errout;
>  	}
> 
> +out:
>  	nlmsg_end(skb, nlh);
>  	return 0;
> 
> --
> 2.50.1 (Apple Git-155)
> 
> 

I see that Checkpatch throws 6 warnings.
Otherwise Patch looks good to me.
Reviewed-by: Sidraya Jayagond <sidraya@linux.ibm.com>