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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 16 Dec 2022 08:41:30 -0800 From: Alexander H Duyck <alexander.duyck@...il.com> To: Jeremy Kerr <jk@...econstruct.com.au>, netdev@...r.kernel.org Cc: Matt Johnston <matt@...econstruct.com.au>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Harsh Tyagi <harshtya@...gle.com> Subject: Re: [PATCH net] mctp: serial: Fix starting value for frame check sequence On Fri, 2022-12-16 at 11:44 +0800, Jeremy Kerr wrote: > RFC1662 defines the start state for the crc16 FCS to be 0xffff, but > we're currently starting at zero. > > This change uses the correct start state. We're only early in the > adoption for the serial binding, so there aren't yet any other users to > interface to. > > Fixes: a0c2ccd9b5ad ("mctp: Add MCTP-over-serial transport binding") > > Reported-by: Harsh Tyagi <harshtya@...gle.com> > Tested-by: Harsh Tyagi <harshtya@...gle.com> > Signed-off-by: Jeremy Kerr <jk@...econstruct.com.au> > --- > drivers/net/mctp/mctp-serial.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c > index 7cd103fd34ef..9f9eaf896047 100644 > --- a/drivers/net/mctp/mctp-serial.c > +++ b/drivers/net/mctp/mctp-serial.c > @@ -35,6 +35,8 @@ > #define BYTE_FRAME 0x7e > #define BYTE_ESC 0x7d > > +#define FCS_INIT 0xffff > + > static DEFINE_IDA(mctp_serial_ida); > > enum mctp_serial_state { > @@ -123,7 +125,7 @@ static void mctp_serial_tx_work(struct work_struct *work) > buf[2] = dev->txlen; > > if (!dev->txpos) > - dev->txfcs = crc_ccitt(0, buf + 1, 2); > + dev->txfcs = crc_ccitt(FCS_INIT, buf + 1, 2); > > txlen = write_chunk(dev, buf + dev->txpos, 3 - dev->txpos); > if (txlen <= 0) { > @@ -303,7 +305,7 @@ static void mctp_serial_push_header(struct mctp_serial *dev, unsigned char c) > case 1: > if (c == MCTP_SERIAL_VERSION) { > dev->rxpos++; > - dev->rxfcs = crc_ccitt_byte(0, c); > + dev->rxfcs = crc_ccitt_byte(FCS_INIT, c); > } else { > dev->rxstate = STATE_ERR; > } Since the starting value isn't unique would it possibly be worthwhile to look at adding a define to include/linux/crc-ccitt.h to be used to handle the cases where the initial value is 0xffff? I notice there seems to only be two starting values 0 and 0xffff for all callers so it might make sense to centralize it in one place. Otherwise the code itself looks good. Reviewed-by: Alexander Duyck <alexanderduyck@...com>
Powered by blists - more mailing lists