diff mbox series

[SRU,F,G,1/1] net/smc: tolerate future SMCD versions

Message ID 20200710192145.203558-2-frank.heimes@canonical.com
State New
Headers show
Series smc: SMC connections hang with later-level implementations (LP: 1882088) | expand

Commit Message

Frank Heimes July 10, 2020, 7:21 p.m. UTC 
From: Ursula Braun <ubraun@linux.ibm.com>

BugLink: https://bugs.launchpad.net/bugs/1882088

CLC proposal messages of future SMCD versions could be larger than SMCD
V1 CLC proposal messages.
To enable toleration in SMC V1 the receival of CLC proposal messages
is adapted:
* accept larger length values in CLC proposal
* check trailing eye catcher for incoming CLC proposal with V1 length only
* receive the whole CLC proposal even in cases it does not fit into the
  V1 buffer

Fixes: e7b7a64a8493d ("smc: support variable CLC proposal messages")
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit fb4f79264c0fc6fd5a68ffe3e31bfff97311e1f1i linux-next)
Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
---
 net/smc/smc_clc.c | 45 ++++++++++++++++++++++++++++++++-------------
 net/smc/smc_clc.h |  4 ++++
 2 files changed, 36 insertions(+), 13 deletions(-)

Comments

Stefan Bader July 13, 2020, 7:20 a.m. UTC | #1 
On 10.07.20 21:21, frank.heimes@canonical.com wrote:
> From: Ursula Braun <ubraun@linux.ibm.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1882088
> 
> CLC proposal messages of future SMCD versions could be larger than SMCD
> V1 CLC proposal messages.
> To enable toleration in SMC V1 the receival of CLC proposal messages
> is adapted:
> * accept larger length values in CLC proposal
> * check trailing eye catcher for incoming CLC proposal with V1 length only
> * receive the whole CLC proposal even in cases it does not fit into the
>   V1 buffer
> 
> Fixes: e7b7a64a8493d ("smc: support variable CLC proposal messages")
> Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
> Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit fb4f79264c0fc6fd5a68ffe3e31bfff97311e1f1i linux-next)
> Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

I am a bit torn here. Though not extremely large, I find it hard to decide about
risk based on the delta. And it is common code that is changed. On the positive
side it is queued upstream, but that not necessarily means it is without flaws.
Have been (or will there be) tests of the new code using the old version (if
that is possible), old and new, and both new?

-Stefan

>  net/smc/smc_clc.c | 45 ++++++++++++++++++++++++w ++++++++-------------
>  net/smc/smc_clc.h |  4 ++++
>  2 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index aee9ccfa99c2..640cce262015 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -27,6 +27,7 @@
>  
>  #define SMCR_CLC_ACCEPT_CONFIRM_LEN 68
>  #define SMCD_CLC_ACCEPT_CONFIRM_LEN 48
> +#define SMC_CLC_RECV_BUF_LEN	100
>  
>  /* eye catcher "SMCR" EBCDIC for CLC messages */
>  static const char SMC_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xd9'};
> @@ -36,7 +37,7 @@ static const char SMCD_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xc4'};
>  /* check if received message has a correct header length and contains valid
>   * heading and trailing eyecatchers
>   */
> -static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
> +static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
>  {
>  	struct smc_clc_msg_proposal_prefix *pclc_prfx;
>  	struct smc_clc_msg_accept_confirm *clc;
> @@ -49,12 +50,9 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
>  		return false;
>  	switch (clcm->type) {
>  	case SMC_CLC_PROPOSAL:
> -		if (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> -		    clcm->path != SMC_TYPE_B)
> -			return false;
>  		pclc = (struct smc_clc_msg_proposal *)clcm;
>  		pclc_prfx = smc_clc_proposal_get_prefix(pclc);
> -		if (ntohs(pclc->hdr.length) !=
> +		if (ntohs(pclc->hdr.length) <
>  			sizeof(*pclc) + ntohs(pclc->iparea_offset) +
>  			sizeof(*pclc_prfx) +
>  			pclc_prfx->ipv6_prefixes_cnt *
> @@ -86,7 +84,8 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
>  	default:
>  		return false;
>  	}
> -	if (memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
> +	if (check_trl &&
> +	    memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
>  	    memcmp(trl->eyecatcher, SMCD_EYECATCHER, sizeof(SMCD_EYECATCHER)))
>  		return false;
>  	return true;
> @@ -276,7 +275,8 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  	struct msghdr msg = {NULL, 0};
>  	int reason_code = 0;
>  	struct kvec vec = {buf, buflen};
> -	int len, datlen;
> +	int len, datlen, recvlen;
> +	bool check_trl = true;
>  	int krflags;
>  
>  	/* peek the first few bytes to determine length of data to receive
> @@ -320,10 +320,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  	}
>  	datlen = ntohs(clcm->length);
>  	if ((len < sizeof(struct smc_clc_msg_hdr)) ||
> -	    (datlen > buflen) ||
> -	    (clcm->version != SMC_CLC_V1) ||
> -	    (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> -	     clcm->path != SMC_TYPE_B) ||
> +	    (clcm->version < SMC_CLC_V1) ||
>  	    ((clcm->type != SMC_CLC_DECLINE) &&
>  	     (clcm->type != expected_type))) {
>  		smc->sk.sk_err = EPROTO;
> @@ -331,16 +328,38 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  		goto out;
>  	}
>  
> +	if (clcm->type == SMC_CLC_PROPOSAL && clcm->path == SMC_TYPE_N)
> +		reason_code = SMC_CLC_DECL_VERSMISMAT; /* just V2 offered */
> +
>  	/* receive the complete CLC message */
>  	memset(&msg, 0, sizeof(struct msghdr));
> -	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, datlen);
> +	if (datlen > buflen) {
> +		check_trl = false;
> +		recvlen = buflen;
> +	} else {
> +		recvlen = datlen;
> +	}
> +	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
>  	krflags = MSG_WAITALL;
>  	len = sock_recvmsg(smc->clcsock, &msg, krflags);
> -	if (len < datlen || !smc_clc_msg_hdr_valid(clcm)) {
> +	if (len < recvlen || !smc_clc_msg_hdr_valid(clcm, check_trl)) {
>  		smc->sk.sk_err = EPROTO;
>  		reason_code = -EPROTO;
>  		goto out;
>  	}
> +	datlen -= len;
> +	while (datlen) {
> +		u8 tmp[SMC_CLC_RECV_BUF_LEN];
> +
> +		vec.iov_base = &tmp;
> +		vec.iov_len = SMC_CLC_RECV_BUF_LEN;
> +		/* receive remaining proposal message */
> +		recvlen = datlen > SMC_CLC_RECV_BUF_LEN ?
> +						SMC_CLC_RECV_BUF_LEN : datlen;
> +		iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
> +		len = sock_recvmsg(smc->clcsock, &msg, krflags);
> +		datlen -= len;
> +	}
>  	if (clcm->type == SMC_CLC_DECLINE) {
>  		struct smc_clc_msg_decline *dclc;
>  
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index ca209272e5fa..76c2b150d040 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -25,6 +25,7 @@
>  #define SMC_CLC_V1		0x1		/* SMC version                */
>  #define SMC_TYPE_R		0		/* SMC-R only		      */
>  #define SMC_TYPE_D		1		/* SMC-D only		      */
> +#define SMC_TYPE_N		2		/* neither SMC-R nor SMC-D    */
>  #define SMC_TYPE_B		3		/* SMC-R and SMC-D	      */
>  #define CLC_WAIT_TIME		(6 * HZ)	/* max. wait time on clcsock  */
>  #define CLC_WAIT_TIME_SHORT	HZ		/* short wait time on clcsock */
> @@ -44,6 +45,9 @@
>  #define SMC_CLC_DECL_DIFFPREFIX	0x03070000  /* IP prefix / subnet mismatch    */
>  #define SMC_CLC_DECL_GETVLANERR	0x03080000  /* err to get vlan id of ip device*/
>  #define SMC_CLC_DECL_ISMVLANERR	0x03090000  /* err to reg vlan id on ism dev  */
> +#define SMC_CLC_DECL_NOACTLINK	0x030a0000  /* no active smc-r link in lgr    */
> +#define SMC_CLC_DECL_NOSRVLINK	0x030b0000  /* SMC-R link from srv not found  */
> +#define SMC_CLC_DECL_VERSMISMAT	0x030c0000  /* SMC version mismatch	      */
>  #define SMC_CLC_DECL_SYNCERR	0x04000000  /* synchronization error          */
>  #define SMC_CLC_DECL_PEERDECL	0x05000000  /* peer declined during handshake */
>  #define SMC_CLC_DECL_INTERR	0x09990000  /* internal error		      */
>
Frank Heimes July 13, 2020, 9:25 a.m. UTC | #2 
While testing the old code, IBM identified the issue and came up with that
patch, that they are now bringing upstream.
The patch was already tested by IBM, but
I provided a patched kernel based on Ubuntu (focal) master-next on top for
further testing:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1882088/comments/12
and I'm expecting some feedback from the initial reporter (IBM) on that,
too.

Thx, Frank


On Mon, Jul 13, 2020 at 9:20 AM Stefan Bader <stefan.bader@canonical.com>
wrote:

> On 10.07.20 21:21, frank.heimes@canonical.com wrote:
> > From: Ursula Braun <ubraun@linux.ibm.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1882088
> >
> > CLC proposal messages of future SMCD versions could be larger than SMCD
> > V1 CLC proposal messages.
> > To enable toleration in SMC V1 the receival of CLC proposal messages
> > is adapted:
> > * accept larger length values in CLC proposal
> > * check trailing eye catcher for incoming CLC proposal with V1 length
> only
> > * receive the whole CLC proposal even in cases it does not fit into the
> >   V1 buffer
> >
> > Fixes: e7b7a64a8493d ("smc: support variable CLC proposal messages")
> > Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
> > Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > (cherry picked from commit fb4f79264c0fc6fd5a68ffe3e31bfff97311e1f1i
> linux-next)
> > Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
> > ---
>
> I am a bit torn here. Though not extremely large, I find it hard to decide
> about
> risk based on the delta. And it is common code that is changed. On the
> positive
> side it is queued upstream, but that not necessarily means it is without
> flaws.
> Have been (or will there be) tests of the new code using the old version
> (if
> that is possible), old and new, and both new?
>
> -Stefan
>
> >  net/smc/smc_clc.c | 45 ++++++++++++++++++++++++w ++++++++-------------
> >  net/smc/smc_clc.h |  4 ++++
> >  2 files changed, 36 insertions(+), 13 deletions(-)
> >
> > diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> > index aee9ccfa99c2..640cce262015 100644
> > --- a/net/smc/smc_clc.c
> > +++ b/net/smc/smc_clc.c
> > @@ -27,6 +27,7 @@
> >
> >  #define SMCR_CLC_ACCEPT_CONFIRM_LEN 68
> >  #define SMCD_CLC_ACCEPT_CONFIRM_LEN 48
> > +#define SMC_CLC_RECV_BUF_LEN 100
> >
> >  /* eye catcher "SMCR" EBCDIC for CLC messages */
> >  static const char SMC_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xd9'};
> > @@ -36,7 +37,7 @@ static const char SMCD_EYECATCHER[4] = {'\xe2',
> '\xd4', '\xc3', '\xc4'};
> >  /* check if received message has a correct header length and contains
> valid
> >   * heading and trailing eyecatchers
> >   */
> > -static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
> > +static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool
> check_trl)
> >  {
> >       struct smc_clc_msg_proposal_prefix *pclc_prfx;
> >       struct smc_clc_msg_accept_confirm *clc;
> > @@ -49,12 +50,9 @@ static bool smc_clc_msg_hdr_valid(struct
> smc_clc_msg_hdr *clcm)
> >               return false;
> >       switch (clcm->type) {
> >       case SMC_CLC_PROPOSAL:
> > -             if (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> > -                 clcm->path != SMC_TYPE_B)
> > -                     return false;
> >               pclc = (struct smc_clc_msg_proposal *)clcm;
> >               pclc_prfx = smc_clc_proposal_get_prefix(pclc);
> > -             if (ntohs(pclc->hdr.length) !=
> > +             if (ntohs(pclc->hdr.length) <
> >                       sizeof(*pclc) + ntohs(pclc->iparea_offset) +
> >                       sizeof(*pclc_prfx) +
> >                       pclc_prfx->ipv6_prefixes_cnt *
> > @@ -86,7 +84,8 @@ static bool smc_clc_msg_hdr_valid(struct
> smc_clc_msg_hdr *clcm)
> >       default:
> >               return false;
> >       }
> > -     if (memcmp(trl->eyecatcher, SMC_EYECATCHER,
> sizeof(SMC_EYECATCHER)) &&
> > +     if (check_trl &&
> > +         memcmp(trl->eyecatcher, SMC_EYECATCHER,
> sizeof(SMC_EYECATCHER)) &&
> >           memcmp(trl->eyecatcher, SMCD_EYECATCHER,
> sizeof(SMCD_EYECATCHER)))
> >               return false;
> >       return true;
> > @@ -276,7 +275,8 @@ int smc_clc_wait_msg(struct smc_sock *smc, void
> *buf, int buflen,
> >       struct msghdr msg = {NULL, 0};
> >       int reason_code = 0;
> >       struct kvec vec = {buf, buflen};
> > -     int len, datlen;
> > +     int len, datlen, recvlen;
> > +     bool check_trl = true;
> >       int krflags;
> >
> >       /* peek the first few bytes to determine length of data to receive
> > @@ -320,10 +320,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void
> *buf, int buflen,
> >       }
> >       datlen = ntohs(clcm->length);
> >       if ((len < sizeof(struct smc_clc_msg_hdr)) ||
> > -         (datlen > buflen) ||
> > -         (clcm->version != SMC_CLC_V1) ||
> > -         (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> > -          clcm->path != SMC_TYPE_B) ||
> > +         (clcm->version < SMC_CLC_V1) ||
> >           ((clcm->type != SMC_CLC_DECLINE) &&
> >            (clcm->type != expected_type))) {
> >               smc->sk.sk_err = EPROTO;
> > @@ -331,16 +328,38 @@ int smc_clc_wait_msg(struct smc_sock *smc, void
> *buf, int buflen,
> >               goto out;
> >       }
> >
> > +     if (clcm->type == SMC_CLC_PROPOSAL && clcm->path == SMC_TYPE_N)
> > +             reason_code = SMC_CLC_DECL_VERSMISMAT; /* just V2 offered
> */
> > +
> >       /* receive the complete CLC message */
> >       memset(&msg, 0, sizeof(struct msghdr));
> > -     iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, datlen);
> > +     if (datlen > buflen) {
> > +             check_trl = false;
> > +             recvlen = buflen;
> > +     } else {
> > +             recvlen = datlen;
> > +     }
> > +     iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
> >       krflags = MSG_WAITALL;
> >       len = sock_recvmsg(smc->clcsock, &msg, krflags);
> > -     if (len < datlen || !smc_clc_msg_hdr_valid(clcm)) {
> > +     if (len < recvlen || !smc_clc_msg_hdr_valid(clcm, check_trl)) {
> >               smc->sk.sk_err = EPROTO;
> >               reason_code = -EPROTO;
> >               goto out;
> >       }
> > +     datlen -= len;
> > +     while (datlen) {
> > +             u8 tmp[SMC_CLC_RECV_BUF_LEN];
> > +
> > +             vec.iov_base = &tmp;
> > +             vec.iov_len = SMC_CLC_RECV_BUF_LEN;
> > +             /* receive remaining proposal message */
> > +             recvlen = datlen > SMC_CLC_RECV_BUF_LEN ?
> > +                                             SMC_CLC_RECV_BUF_LEN :
> datlen;
> > +             iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
> > +             len = sock_recvmsg(smc->clcsock, &msg, krflags);
> > +             datlen -= len;
> > +     }
> >       if (clcm->type == SMC_CLC_DECLINE) {
> >               struct smc_clc_msg_decline *dclc;
> >
> > diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> > index ca209272e5fa..76c2b150d040 100644
> > --- a/net/smc/smc_clc.h
> > +++ b/net/smc/smc_clc.h
> > @@ -25,6 +25,7 @@
> >  #define SMC_CLC_V1           0x1             /* SMC version
>     */
> >  #define SMC_TYPE_R           0               /* SMC-R only
>    */
> >  #define SMC_TYPE_D           1               /* SMC-D only
>    */
> > +#define SMC_TYPE_N           2               /* neither SMC-R nor
> SMC-D    */
> >  #define SMC_TYPE_B           3               /* SMC-R and SMC-D
>     */
> >  #define CLC_WAIT_TIME                (6 * HZ)        /* max. wait time
> on clcsock  */
> >  #define CLC_WAIT_TIME_SHORT  HZ              /* short wait time on
> clcsock */
> > @@ -44,6 +45,9 @@
> >  #define SMC_CLC_DECL_DIFFPREFIX      0x03070000  /* IP prefix / subnet
> mismatch    */
> >  #define SMC_CLC_DECL_GETVLANERR      0x03080000  /* err to get vlan id
> of ip device*/
> >  #define SMC_CLC_DECL_ISMVLANERR      0x03090000  /* err to reg vlan id
> on ism dev  */
> > +#define SMC_CLC_DECL_NOACTLINK       0x030a0000  /* no active smc-r
> link in lgr    */
> > +#define SMC_CLC_DECL_NOSRVLINK       0x030b0000  /* SMC-R link from srv
> not found  */
> > +#define SMC_CLC_DECL_VERSMISMAT      0x030c0000  /* SMC version
> mismatch           */
> >  #define SMC_CLC_DECL_SYNCERR 0x04000000  /* synchronization error
>     */
> >  #define SMC_CLC_DECL_PEERDECL        0x05000000  /* peer declined
> during handshake */
> >  #define SMC_CLC_DECL_INTERR  0x09990000  /* internal error
>    */
> >
>
>
>
Kleber Sacilotto de Souza July 14, 2020, 10:06 a.m. UTC | #3 
On 10.07.20 21:21, frank.heimes@canonical.com wrote:
> From: Ursula Braun <ubraun@linux.ibm.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1882088
> 
> CLC proposal messages of future SMCD versions could be larger than SMCD
> V1 CLC proposal messages.
> To enable toleration in SMC V1 the receival of CLC proposal messages
> is adapted:
> * accept larger length values in CLC proposal
> * check trailing eye catcher for incoming CLC proposal with V1 length only
> * receive the whole CLC proposal even in cases it does not fit into the
>   V1 buffer
> 
> Fixes: e7b7a64a8493d ("smc: support variable CLC proposal messages")
> Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
> Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit fb4f79264c0fc6fd5a68ffe3e31bfff97311e1f1i linux-next)

There's a bogus "i" at the end of the SHA1 (vim user detected? :) ^

Apart from that it looks good to me.

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
> ---
>  net/smc/smc_clc.c | 45 ++++++++++++++++++++++++++++++++-------------
>  net/smc/smc_clc.h |  4 ++++
>  2 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index aee9ccfa99c2..640cce262015 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -27,6 +27,7 @@
>  
>  #define SMCR_CLC_ACCEPT_CONFIRM_LEN 68
>  #define SMCD_CLC_ACCEPT_CONFIRM_LEN 48
> +#define SMC_CLC_RECV_BUF_LEN	100
>  
>  /* eye catcher "SMCR" EBCDIC for CLC messages */
>  static const char SMC_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xd9'};
> @@ -36,7 +37,7 @@ static const char SMCD_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xc4'};
>  /* check if received message has a correct header length and contains valid
>   * heading and trailing eyecatchers
>   */
> -static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
> +static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
>  {
>  	struct smc_clc_msg_proposal_prefix *pclc_prfx;
>  	struct smc_clc_msg_accept_confirm *clc;
> @@ -49,12 +50,9 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
>  		return false;
>  	switch (clcm->type) {
>  	case SMC_CLC_PROPOSAL:
> -		if (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> -		    clcm->path != SMC_TYPE_B)
> -			return false;
>  		pclc = (struct smc_clc_msg_proposal *)clcm;
>  		pclc_prfx = smc_clc_proposal_get_prefix(pclc);
> -		if (ntohs(pclc->hdr.length) !=
> +		if (ntohs(pclc->hdr.length) <
>  			sizeof(*pclc) + ntohs(pclc->iparea_offset) +
>  			sizeof(*pclc_prfx) +
>  			pclc_prfx->ipv6_prefixes_cnt *
> @@ -86,7 +84,8 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
>  	default:
>  		return false;
>  	}
> -	if (memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
> +	if (check_trl &&
> +	    memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
>  	    memcmp(trl->eyecatcher, SMCD_EYECATCHER, sizeof(SMCD_EYECATCHER)))
>  		return false;
>  	return true;
> @@ -276,7 +275,8 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  	struct msghdr msg = {NULL, 0};
>  	int reason_code = 0;
>  	struct kvec vec = {buf, buflen};
> -	int len, datlen;
> +	int len, datlen, recvlen;
> +	bool check_trl = true;
>  	int krflags;
>  
>  	/* peek the first few bytes to determine length of data to receive
> @@ -320,10 +320,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  	}
>  	datlen = ntohs(clcm->length);
>  	if ((len < sizeof(struct smc_clc_msg_hdr)) ||
> -	    (datlen > buflen) ||
> -	    (clcm->version != SMC_CLC_V1) ||
> -	    (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> -	     clcm->path != SMC_TYPE_B) ||
> +	    (clcm->version < SMC_CLC_V1) ||
>  	    ((clcm->type != SMC_CLC_DECLINE) &&
>  	     (clcm->type != expected_type))) {
>  		smc->sk.sk_err = EPROTO;
> @@ -331,16 +328,38 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  		goto out;
>  	}
>  
> +	if (clcm->type == SMC_CLC_PROPOSAL && clcm->path == SMC_TYPE_N)
> +		reason_code = SMC_CLC_DECL_VERSMISMAT; /* just V2 offered */
> +
>  	/* receive the complete CLC message */
>  	memset(&msg, 0, sizeof(struct msghdr));
> -	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, datlen);
> +	if (datlen > buflen) {
> +		check_trl = false;
> +		recvlen = buflen;
> +	} else {
> +		recvlen = datlen;
> +	}
> +	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
>  	krflags = MSG_WAITALL;
>  	len = sock_recvmsg(smc->clcsock, &msg, krflags);
> -	if (len < datlen || !smc_clc_msg_hdr_valid(clcm)) {
> +	if (len < recvlen || !smc_clc_msg_hdr_valid(clcm, check_trl)) {
>  		smc->sk.sk_err = EPROTO;
>  		reason_code = -EPROTO;
>  		goto out;
>  	}
> +	datlen -= len;
> +	while (datlen) {
> +		u8 tmp[SMC_CLC_RECV_BUF_LEN];
> +
> +		vec.iov_base = &tmp;
> +		vec.iov_len = SMC_CLC_RECV_BUF_LEN;
> +		/* receive remaining proposal message */
> +		recvlen = datlen > SMC_CLC_RECV_BUF_LEN ?
> +						SMC_CLC_RECV_BUF_LEN : datlen;
> +		iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
> +		len = sock_recvmsg(smc->clcsock, &msg, krflags);
> +		datlen -= len;
> +	}
>  	if (clcm->type == SMC_CLC_DECLINE) {
>  		struct smc_clc_msg_decline *dclc;
>  
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index ca209272e5fa..76c2b150d040 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -25,6 +25,7 @@
>  #define SMC_CLC_V1		0x1		/* SMC version                */
>  #define SMC_TYPE_R		0		/* SMC-R only		      */
>  #define SMC_TYPE_D		1		/* SMC-D only		      */
> +#define SMC_TYPE_N		2		/* neither SMC-R nor SMC-D    */
>  #define SMC_TYPE_B		3		/* SMC-R and SMC-D	      */
>  #define CLC_WAIT_TIME		(6 * HZ)	/* max. wait time on clcsock  */
>  #define CLC_WAIT_TIME_SHORT	HZ		/* short wait time on clcsock */
> @@ -44,6 +45,9 @@
>  #define SMC_CLC_DECL_DIFFPREFIX	0x03070000  /* IP prefix / subnet mismatch    */
>  #define SMC_CLC_DECL_GETVLANERR	0x03080000  /* err to get vlan id of ip device*/
>  #define SMC_CLC_DECL_ISMVLANERR	0x03090000  /* err to reg vlan id on ism dev  */
> +#define SMC_CLC_DECL_NOACTLINK	0x030a0000  /* no active smc-r link in lgr    */
> +#define SMC_CLC_DECL_NOSRVLINK	0x030b0000  /* SMC-R link from srv not found  */
> +#define SMC_CLC_DECL_VERSMISMAT	0x030c0000  /* SMC version mismatch	      */
>  #define SMC_CLC_DECL_SYNCERR	0x04000000  /* synchronization error          */
>  #define SMC_CLC_DECL_PEERDECL	0x05000000  /* peer declined during handshake */
>  #define SMC_CLC_DECL_INTERR	0x09990000  /* internal error		      */
>
Seth Forshee July 17, 2020, 10:33 p.m. UTC | #4 
On Fri, Jul 10, 2020 at 09:21:45PM +0200, frank.heimes@canonical.com wrote:
> From: Ursula Braun <ubraun@linux.ibm.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1882088
> 
> CLC proposal messages of future SMCD versions could be larger than SMCD
> V1 CLC proposal messages.
> To enable toleration in SMC V1 the receival of CLC proposal messages
> is adapted:
> * accept larger length values in CLC proposal
> * check trailing eye catcher for incoming CLC proposal with V1 length only
> * receive the whole CLC proposal even in cases it does not fit into the
>   V1 buffer
> 
> Fixes: e7b7a64a8493d ("smc: support variable CLC proposal messages")
> Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
> Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (cherry picked from commit fb4f79264c0fc6fd5a68ffe3e31bfff97311e1f1i linux-next)

One note, there is an 'i' appended to the end of the sha1 which should
be removed when applying.

> Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
> ---
>  net/smc/smc_clc.c | 45 ++++++++++++++++++++++++++++++++-------------
>  net/smc/smc_clc.h |  4 ++++
>  2 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
> index aee9ccfa99c2..640cce262015 100644
> --- a/net/smc/smc_clc.c
> +++ b/net/smc/smc_clc.c
> @@ -27,6 +27,7 @@
>  
>  #define SMCR_CLC_ACCEPT_CONFIRM_LEN 68
>  #define SMCD_CLC_ACCEPT_CONFIRM_LEN 48
> +#define SMC_CLC_RECV_BUF_LEN	100
>  
>  /* eye catcher "SMCR" EBCDIC for CLC messages */
>  static const char SMC_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xd9'};
> @@ -36,7 +37,7 @@ static const char SMCD_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xc4'};
>  /* check if received message has a correct header length and contains valid
>   * heading and trailing eyecatchers
>   */
> -static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
> +static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
>  {
>  	struct smc_clc_msg_proposal_prefix *pclc_prfx;
>  	struct smc_clc_msg_accept_confirm *clc;
> @@ -49,12 +50,9 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
>  		return false;
>  	switch (clcm->type) {
>  	case SMC_CLC_PROPOSAL:
> -		if (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> -		    clcm->path != SMC_TYPE_B)
> -			return false;
>  		pclc = (struct smc_clc_msg_proposal *)clcm;
>  		pclc_prfx = smc_clc_proposal_get_prefix(pclc);
> -		if (ntohs(pclc->hdr.length) !=
> +		if (ntohs(pclc->hdr.length) <
>  			sizeof(*pclc) + ntohs(pclc->iparea_offset) +
>  			sizeof(*pclc_prfx) +
>  			pclc_prfx->ipv6_prefixes_cnt *
> @@ -86,7 +84,8 @@ static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
>  	default:
>  		return false;
>  	}
> -	if (memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
> +	if (check_trl &&
> +	    memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
>  	    memcmp(trl->eyecatcher, SMCD_EYECATCHER, sizeof(SMCD_EYECATCHER)))
>  		return false;
>  	return true;
> @@ -276,7 +275,8 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  	struct msghdr msg = {NULL, 0};
>  	int reason_code = 0;
>  	struct kvec vec = {buf, buflen};
> -	int len, datlen;
> +	int len, datlen, recvlen;
> +	bool check_trl = true;
>  	int krflags;
>  
>  	/* peek the first few bytes to determine length of data to receive
> @@ -320,10 +320,7 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  	}
>  	datlen = ntohs(clcm->length);
>  	if ((len < sizeof(struct smc_clc_msg_hdr)) ||
> -	    (datlen > buflen) ||
> -	    (clcm->version != SMC_CLC_V1) ||
> -	    (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
> -	     clcm->path != SMC_TYPE_B) ||
> +	    (clcm->version < SMC_CLC_V1) ||
>  	    ((clcm->type != SMC_CLC_DECLINE) &&
>  	     (clcm->type != expected_type))) {
>  		smc->sk.sk_err = EPROTO;
> @@ -331,16 +328,38 @@ int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
>  		goto out;
>  	}
>  
> +	if (clcm->type == SMC_CLC_PROPOSAL && clcm->path == SMC_TYPE_N)
> +		reason_code = SMC_CLC_DECL_VERSMISMAT; /* just V2 offered */
> +
>  	/* receive the complete CLC message */
>  	memset(&msg, 0, sizeof(struct msghdr));
> -	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, datlen);
> +	if (datlen > buflen) {
> +		check_trl = false;
> +		recvlen = buflen;
> +	} else {
> +		recvlen = datlen;
> +	}
> +	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
>  	krflags = MSG_WAITALL;
>  	len = sock_recvmsg(smc->clcsock, &msg, krflags);
> -	if (len < datlen || !smc_clc_msg_hdr_valid(clcm)) {
> +	if (len < recvlen || !smc_clc_msg_hdr_valid(clcm, check_trl)) {
>  		smc->sk.sk_err = EPROTO;
>  		reason_code = -EPROTO;
>  		goto out;
>  	}
> +	datlen -= len;
> +	while (datlen) {
> +		u8 tmp[SMC_CLC_RECV_BUF_LEN];
> +
> +		vec.iov_base = &tmp;
> +		vec.iov_len = SMC_CLC_RECV_BUF_LEN;
> +		/* receive remaining proposal message */
> +		recvlen = datlen > SMC_CLC_RECV_BUF_LEN ?
> +						SMC_CLC_RECV_BUF_LEN : datlen;
> +		iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
> +		len = sock_recvmsg(smc->clcsock, &msg, krflags);
> +		datlen -= len;
> +	}
>  	if (clcm->type == SMC_CLC_DECLINE) {
>  		struct smc_clc_msg_decline *dclc;
>  
> diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
> index ca209272e5fa..76c2b150d040 100644
> --- a/net/smc/smc_clc.h
> +++ b/net/smc/smc_clc.h
> @@ -25,6 +25,7 @@
>  #define SMC_CLC_V1		0x1		/* SMC version                */
>  #define SMC_TYPE_R		0		/* SMC-R only		      */
>  #define SMC_TYPE_D		1		/* SMC-D only		      */
> +#define SMC_TYPE_N		2		/* neither SMC-R nor SMC-D    */
>  #define SMC_TYPE_B		3		/* SMC-R and SMC-D	      */
>  #define CLC_WAIT_TIME		(6 * HZ)	/* max. wait time on clcsock  */
>  #define CLC_WAIT_TIME_SHORT	HZ		/* short wait time on clcsock */
> @@ -44,6 +45,9 @@
>  #define SMC_CLC_DECL_DIFFPREFIX	0x03070000  /* IP prefix / subnet mismatch    */
>  #define SMC_CLC_DECL_GETVLANERR	0x03080000  /* err to get vlan id of ip device*/
>  #define SMC_CLC_DECL_ISMVLANERR	0x03090000  /* err to reg vlan id on ism dev  */
> +#define SMC_CLC_DECL_NOACTLINK	0x030a0000  /* no active smc-r link in lgr    */
> +#define SMC_CLC_DECL_NOSRVLINK	0x030b0000  /* SMC-R link from srv not found  */
> +#define SMC_CLC_DECL_VERSMISMAT	0x030c0000  /* SMC version mismatch	      */
>  #define SMC_CLC_DECL_SYNCERR	0x04000000  /* synchronization error          */
>  #define SMC_CLC_DECL_PEERDECL	0x05000000  /* peer declined during handshake */
>  #define SMC_CLC_DECL_INTERR	0x09990000  /* internal error		      */
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index aee9ccfa99c2..640cce262015 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -27,6 +27,7 @@ 
 
 #define SMCR_CLC_ACCEPT_CONFIRM_LEN 68
 #define SMCD_CLC_ACCEPT_CONFIRM_LEN 48
+#define SMC_CLC_RECV_BUF_LEN	100
 
 /* eye catcher "SMCR" EBCDIC for CLC messages */
 static const char SMC_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xd9'};
@@ -36,7 +37,7 @@  static const char SMCD_EYECATCHER[4] = {'\xe2', '\xd4', '\xc3', '\xc4'};
 /* check if received message has a correct header length and contains valid
  * heading and trailing eyecatchers
  */
-static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
+static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm, bool check_trl)
 {
 	struct smc_clc_msg_proposal_prefix *pclc_prfx;
 	struct smc_clc_msg_accept_confirm *clc;
@@ -49,12 +50,9 @@  static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
 		return false;
 	switch (clcm->type) {
 	case SMC_CLC_PROPOSAL:
-		if (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
-		    clcm->path != SMC_TYPE_B)
-			return false;
 		pclc = (struct smc_clc_msg_proposal *)clcm;
 		pclc_prfx = smc_clc_proposal_get_prefix(pclc);
-		if (ntohs(pclc->hdr.length) !=
+		if (ntohs(pclc->hdr.length) <
 			sizeof(*pclc) + ntohs(pclc->iparea_offset) +
 			sizeof(*pclc_prfx) +
 			pclc_prfx->ipv6_prefixes_cnt *
@@ -86,7 +84,8 @@  static bool smc_clc_msg_hdr_valid(struct smc_clc_msg_hdr *clcm)
 	default:
 		return false;
 	}
-	if (memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
+	if (check_trl &&
+	    memcmp(trl->eyecatcher, SMC_EYECATCHER, sizeof(SMC_EYECATCHER)) &&
 	    memcmp(trl->eyecatcher, SMCD_EYECATCHER, sizeof(SMCD_EYECATCHER)))
 		return false;
 	return true;
@@ -276,7 +275,8 @@  int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 	struct msghdr msg = {NULL, 0};
 	int reason_code = 0;
 	struct kvec vec = {buf, buflen};
-	int len, datlen;
+	int len, datlen, recvlen;
+	bool check_trl = true;
 	int krflags;
 
 	/* peek the first few bytes to determine length of data to receive
@@ -320,10 +320,7 @@  int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 	}
 	datlen = ntohs(clcm->length);
 	if ((len < sizeof(struct smc_clc_msg_hdr)) ||
-	    (datlen > buflen) ||
-	    (clcm->version != SMC_CLC_V1) ||
-	    (clcm->path != SMC_TYPE_R && clcm->path != SMC_TYPE_D &&
-	     clcm->path != SMC_TYPE_B) ||
+	    (clcm->version < SMC_CLC_V1) ||
 	    ((clcm->type != SMC_CLC_DECLINE) &&
 	     (clcm->type != expected_type))) {
 		smc->sk.sk_err = EPROTO;
@@ -331,16 +328,38 @@  int smc_clc_wait_msg(struct smc_sock *smc, void *buf, int buflen,
 		goto out;
 	}
 
+	if (clcm->type == SMC_CLC_PROPOSAL && clcm->path == SMC_TYPE_N)
+		reason_code = SMC_CLC_DECL_VERSMISMAT; /* just V2 offered */
+
 	/* receive the complete CLC message */
 	memset(&msg, 0, sizeof(struct msghdr));
-	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, datlen);
+	if (datlen > buflen) {
+		check_trl = false;
+		recvlen = buflen;
+	} else {
+		recvlen = datlen;
+	}
+	iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
 	krflags = MSG_WAITALL;
 	len = sock_recvmsg(smc->clcsock, &msg, krflags);
-	if (len < datlen || !smc_clc_msg_hdr_valid(clcm)) {
+	if (len < recvlen || !smc_clc_msg_hdr_valid(clcm, check_trl)) {
 		smc->sk.sk_err = EPROTO;
 		reason_code = -EPROTO;
 		goto out;
 	}
+	datlen -= len;
+	while (datlen) {
+		u8 tmp[SMC_CLC_RECV_BUF_LEN];
+
+		vec.iov_base = &tmp;
+		vec.iov_len = SMC_CLC_RECV_BUF_LEN;
+		/* receive remaining proposal message */
+		recvlen = datlen > SMC_CLC_RECV_BUF_LEN ?
+						SMC_CLC_RECV_BUF_LEN : datlen;
+		iov_iter_kvec(&msg.msg_iter, READ, &vec, 1, recvlen);
+		len = sock_recvmsg(smc->clcsock, &msg, krflags);
+		datlen -= len;
+	}
 	if (clcm->type == SMC_CLC_DECLINE) {
 		struct smc_clc_msg_decline *dclc;
 
diff --git a/net/smc/smc_clc.h b/net/smc/smc_clc.h
index ca209272e5fa..76c2b150d040 100644
--- a/net/smc/smc_clc.h
+++ b/net/smc/smc_clc.h
@@ -25,6 +25,7 @@ 
 #define SMC_CLC_V1		0x1		/* SMC version                */
 #define SMC_TYPE_R		0		/* SMC-R only		      */
 #define SMC_TYPE_D		1		/* SMC-D only		      */
+#define SMC_TYPE_N		2		/* neither SMC-R nor SMC-D    */
 #define SMC_TYPE_B		3		/* SMC-R and SMC-D	      */
 #define CLC_WAIT_TIME		(6 * HZ)	/* max. wait time on clcsock  */
 #define CLC_WAIT_TIME_SHORT	HZ		/* short wait time on clcsock */
@@ -44,6 +45,9 @@ 
 #define SMC_CLC_DECL_DIFFPREFIX	0x03070000  /* IP prefix / subnet mismatch    */
 #define SMC_CLC_DECL_GETVLANERR	0x03080000  /* err to get vlan id of ip device*/
 #define SMC_CLC_DECL_ISMVLANERR	0x03090000  /* err to reg vlan id on ism dev  */
+#define SMC_CLC_DECL_NOACTLINK	0x030a0000  /* no active smc-r link in lgr    */
+#define SMC_CLC_DECL_NOSRVLINK	0x030b0000  /* SMC-R link from srv not found  */
+#define SMC_CLC_DECL_VERSMISMAT	0x030c0000  /* SMC version mismatch	      */
 #define SMC_CLC_DECL_SYNCERR	0x04000000  /* synchronization error          */
 #define SMC_CLC_DECL_PEERDECL	0x05000000  /* peer declined during handshake */
 #define SMC_CLC_DECL_INTERR	0x09990000  /* internal error		      */