* [PATCH net 0/2] net: mctp-serial: Fix for missing tx escapes @ 2024-08-27 2:07 Matt Johnston 2024-08-27 2:07 ` [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() Matt Johnston 2024-08-27 2:07 ` [PATCH net 2/2] net: mctp-serial: Fix missing escapes on transmit Matt Johnston 0 siblings, 2 replies; 6+ messages in thread From: Matt Johnston @ 2024-08-27 2:07 UTC (permalink / raw) To: jk; +Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet, netdev The mctp-serial code to add escape characters was incorrect due to an off-by-one error. This series adds a test for the chunking which splits by escape characters, and fixes the bug. Matt Johnston (2): net: mctp-serial: Add kunit test for next_chunk_len() net: mctp-serial: Fix missing escapes on transmit drivers/net/mctp/Kconfig | 5 ++ drivers/net/mctp/mctp-serial.c | 113 ++++++++++++++++++++++++++++++++- 2 files changed, 116 insertions(+), 2 deletions(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() 2024-08-27 2:07 [PATCH net 0/2] net: mctp-serial: Fix for missing tx escapes Matt Johnston @ 2024-08-27 2:07 ` Matt Johnston 2024-08-27 23:13 ` kernel test robot 2024-08-29 7:30 ` kernel test robot 2024-08-27 2:07 ` [PATCH net 2/2] net: mctp-serial: Fix missing escapes on transmit Matt Johnston 1 sibling, 2 replies; 6+ messages in thread From: Matt Johnston @ 2024-08-27 2:07 UTC (permalink / raw) To: jk; +Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet, netdev Test various edge cases of inputs that contain characters that need escaping. This adds a new kunit suite for mctp-serial. Signed-off-by: Matt Johnston <matt@codeconstruct.com.au> --- drivers/net/mctp/Kconfig | 5 ++ drivers/net/mctp/mctp-serial.c | 109 +++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/drivers/net/mctp/Kconfig b/drivers/net/mctp/Kconfig index ce9d2d2ccf3b..15860d6ac39f 100644 --- a/drivers/net/mctp/Kconfig +++ b/drivers/net/mctp/Kconfig @@ -21,6 +21,11 @@ config MCTP_SERIAL Say y here if you need to connect to MCTP endpoints over serial. To compile as a module, use m; the module will be called mctp-serial. +config MCTP_SERIAL_TEST + bool "MCTP serial tests" if !KUNIT_ALL_TESTS + depends on MCTP_SERIAL=y && KUNIT=y + default KUNIT_ALL_TESTS + config MCTP_TRANSPORT_I2C tristate "MCTP SMBus/I2C transport" # i2c-mux is optional, but we must build as a module if i2c-mux is a module diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c index 5bf6fdff701c..d7db11355909 100644 --- a/drivers/net/mctp/mctp-serial.c +++ b/drivers/net/mctp/mctp-serial.c @@ -521,3 +521,112 @@ module_exit(mctp_serial_exit); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Jeremy Kerr <jk@codeconstruct.com.au>"); MODULE_DESCRIPTION("MCTP Serial transport"); + +#if IS_ENABLED(CONFIG_MCTP_SERIAL_TEST) +#include <kunit/test.h> + +#define MAX_CHUNKS 6 +struct test_chunk_tx { + u8 input_len; + u8 input[MCTP_SERIAL_MTU]; + u8 chunks[MAX_CHUNKS]; +}; + +static void test_next_chunk_len(struct kunit *test) +{ + struct mctp_serial devx; + struct mctp_serial *dev = &devx; + int next; + + struct test_chunk_tx *params = test->param_value; + + memset(dev, 0x0, sizeof(*dev)); + memcpy(dev->txbuf, params->input, params->input_len); + dev->txlen = params->input_len; + + for (size_t i = 0; i < MAX_CHUNKS; i++) { + next = next_chunk_len(dev); + dev->txpos += next; + KUNIT_EXPECT_EQ(test, next, params->chunks[i]); + + if (next == 0) { + KUNIT_EXPECT_EQ(test, dev->txpos, dev->txlen); + return; + } + } + + KUNIT_FAIL_AND_ABORT(test, "Ran out of chunks"); +} + +static struct test_chunk_tx chunk_tx_tests[] = { + { + .input_len = 5, + .input = { 0x00, 0x11, 0x22, 0x7e, 0x80 }, + .chunks = { 3, 1, 1, 0}, + }, + { + .input_len = 5, + .input = { 0x00, 0x11, 0x22, 0x7e, 0x7d }, + .chunks = { 3, 1, 1, 0}, + }, + { + .input_len = 3, + .input = { 0x7e, 0x11, 0x22, }, + .chunks = { 1, 2, 0}, + }, + { + .input_len = 3, + .input = { 0x7e, 0x7e, 0x7d, }, + .chunks = { 1, 1, 1, 0}, + }, + { + .input_len = 4, + .input = { 0x7e, 0x7e, 0x00, 0x7d, }, + .chunks = { 1, 1, 1, 1, 0}, + }, + { + .input_len = 6, + .input = { 0x7e, 0x7e, 0x00, 0x7d, 0x10, 0x10}, + .chunks = { 1, 1, 1, 1, 2, 0}, + }, + { + .input_len = 1, + .input = { 0x7e }, + .chunks = { 1, 0 }, + }, + { + .input_len = 1, + .input = { 0x80 }, + .chunks = { 1, 0 }, + }, + { + .input_len = 3, + .input = { 0x80, 0x80, 0x00 }, + .chunks = { 3, 0 }, + }, + { + .input_len = 7, + .input = { 0x01, 0x00, 0x08, 0xc8, 0x00, 0x80, 0x02 }, + .chunks = { 7, 0 }, + }, + { + .input_len = 7, + .input = { 0x01, 0x00, 0x08, 0xc8, 0x7e, 0x80, 0x02 }, + .chunks = { 4, 1, 2, 0 }, + }, +}; + +KUNIT_ARRAY_PARAM(chunk_tx, chunk_tx_tests, NULL); + +static struct kunit_case mctp_serial_test_cases[] = { + KUNIT_CASE_PARAM(test_next_chunk_len, chunk_tx_gen_params), +}; + +static struct kunit_suite mctp_serial_test_suite = { + .name = "mctp_serial", + .test_cases = mctp_serial_test_cases, +}; + +kunit_test_suite(mctp_serial_test_suite); + +#endif /* CONFIG_MCTP_SERIAL_TEST */ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() 2024-08-27 2:07 ` [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() Matt Johnston @ 2024-08-27 23:13 ` kernel test robot 2024-08-29 7:30 ` kernel test robot 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2024-08-27 23:13 UTC (permalink / raw) To: Matt Johnston, jk Cc: oe-kbuild-all, Jakub Kicinski, Paolo Abeni, Eric Dumazet, netdev Hi Matt, kernel test robot noticed the following build warnings: [auto build test WARNING on net/main] url: https://github.com/intel-lab-lkp/linux/commits/Matt-Johnston/net-mctp-serial-Add-kunit-test-for-next_chunk_len/20240827-101656 base: net/main patch link: https://lore.kernel.org/r/20240827020803.957250-2-matt%40codeconstruct.com.au patch subject: [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() config: s390-allyesconfig (https://download.01.org/0day-ci/archive/20240828/202408280734.ioysUDjL-lkp@intel.com/config) compiler: s390-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280734.ioysUDjL-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408280734.ioysUDjL-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/net/mctp/mctp-serial.c: In function 'test_next_chunk_len': >> drivers/net/mctp/mctp-serial.c:541:40: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] 541 | struct test_chunk_tx *params = test->param_value; | ^~~~ vim +/const +541 drivers/net/mctp/mctp-serial.c 534 535 static void test_next_chunk_len(struct kunit *test) 536 { 537 struct mctp_serial devx; 538 struct mctp_serial *dev = &devx; 539 int next; 540 > 541 struct test_chunk_tx *params = test->param_value; 542 543 memset(dev, 0x0, sizeof(*dev)); 544 memcpy(dev->txbuf, params->input, params->input_len); 545 dev->txlen = params->input_len; 546 547 for (size_t i = 0; i < MAX_CHUNKS; i++) { 548 next = next_chunk_len(dev); 549 dev->txpos += next; 550 KUNIT_EXPECT_EQ(test, next, params->chunks[i]); 551 552 if (next == 0) { 553 KUNIT_EXPECT_EQ(test, dev->txpos, dev->txlen); 554 return; 555 } 556 } 557 558 KUNIT_FAIL_AND_ABORT(test, "Ran out of chunks"); 559 } 560 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() 2024-08-27 2:07 ` [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() Matt Johnston 2024-08-27 23:13 ` kernel test robot @ 2024-08-29 7:30 ` kernel test robot 1 sibling, 0 replies; 6+ messages in thread From: kernel test robot @ 2024-08-29 7:30 UTC (permalink / raw) To: Matt Johnston, jk Cc: llvm, oe-kbuild-all, Jakub Kicinski, Paolo Abeni, Eric Dumazet, netdev Hi Matt, kernel test robot noticed the following build errors: [auto build test ERROR on net/main] url: https://github.com/intel-lab-lkp/linux/commits/Matt-Johnston/net-mctp-serial-Add-kunit-test-for-next_chunk_len/20240827-101656 base: net/main patch link: https://lore.kernel.org/r/20240827020803.957250-2-matt%40codeconstruct.com.au patch subject: [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20240829/202408291542.2o5GOBhQ-lkp@intel.com/config) compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240829/202408291542.2o5GOBhQ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202408291542.2o5GOBhQ-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/net/mctp/mctp-serial.c:541:24: error: initializing 'struct test_chunk_tx *' with an expression of type 'const void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers] 541 | struct test_chunk_tx *params = test->param_value; | ^ ~~~~~~~~~~~~~~~~~ 1 error generated. vim +541 drivers/net/mctp/mctp-serial.c 534 535 static void test_next_chunk_len(struct kunit *test) 536 { 537 struct mctp_serial devx; 538 struct mctp_serial *dev = &devx; 539 int next; 540 > 541 struct test_chunk_tx *params = test->param_value; 542 543 memset(dev, 0x0, sizeof(*dev)); 544 memcpy(dev->txbuf, params->input, params->input_len); 545 dev->txlen = params->input_len; 546 547 for (size_t i = 0; i < MAX_CHUNKS; i++) { 548 next = next_chunk_len(dev); 549 dev->txpos += next; 550 KUNIT_EXPECT_EQ(test, next, params->chunks[i]); 551 552 if (next == 0) { 553 KUNIT_EXPECT_EQ(test, dev->txpos, dev->txlen); 554 return; 555 } 556 } 557 558 KUNIT_FAIL_AND_ABORT(test, "Ran out of chunks"); 559 } 560 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 2/2] net: mctp-serial: Fix missing escapes on transmit 2024-08-27 2:07 [PATCH net 0/2] net: mctp-serial: Fix for missing tx escapes Matt Johnston 2024-08-27 2:07 ` [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() Matt Johnston @ 2024-08-27 2:07 ` Matt Johnston 2024-08-27 11:06 ` Larysa Zaremba 1 sibling, 1 reply; 6+ messages in thread From: Matt Johnston @ 2024-08-27 2:07 UTC (permalink / raw) To: jk Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet, netdev, stable 0x7d and 0x7e bytes are meant to be escaped in the data portion of frames, but this didn't occur since next_chunk_len() had an off-by-one error. That also resulted in the final byte of a payload being written as a separate tty write op. The chunk prior to an escaped byte would be one byte short, and the next call would never test the txpos+1 case, which is where the escaped byte was located. That meant it never hit the escaping case in mctp_serial_tx_work(). Example Input: 01 00 08 c8 7e 80 02 Previous incorrect chunks from next_chunk_len(): 01 00 08 c8 7e 80 02 With this fix: 01 00 08 c8 7e 80 02 Cc: stable@vger.kernel.org Fixes: a0c2ccd9b5ad ("mctp: Add MCTP-over-serial transport binding") Signed-off-by: Matt Johnston <matt@codeconstruct.com.au> --- drivers/net/mctp/mctp-serial.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c index d7db11355909..82890e983847 100644 --- a/drivers/net/mctp/mctp-serial.c +++ b/drivers/net/mctp/mctp-serial.c @@ -91,8 +91,8 @@ static int next_chunk_len(struct mctp_serial *dev) * will be those non-escaped bytes, and does not include the escaped * byte. */ - for (i = 1; i + dev->txpos + 1 < dev->txlen; i++) { - if (needs_escape(dev->txbuf[dev->txpos + i + 1])) + for (i = 1; i + dev->txpos < dev->txlen; i++) { + if (needs_escape(dev->txbuf[dev->txpos + i])) break; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] net: mctp-serial: Fix missing escapes on transmit 2024-08-27 2:07 ` [PATCH net 2/2] net: mctp-serial: Fix missing escapes on transmit Matt Johnston @ 2024-08-27 11:06 ` Larysa Zaremba 0 siblings, 0 replies; 6+ messages in thread From: Larysa Zaremba @ 2024-08-27 11:06 UTC (permalink / raw) To: Matt Johnston Cc: jk, David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet, netdev, stable On Tue, Aug 27, 2024 at 10:07:59AM +0800, Matt Johnston wrote: > 0x7d and 0x7e bytes are meant to be escaped in the data portion of > frames, but this didn't occur since next_chunk_len() had an off-by-one > error. That also resulted in the final byte of a payload being written > as a separate tty write op. > > The chunk prior to an escaped byte would be one byte short, and the > next call would never test the txpos+1 case, which is where the escaped > byte was located. That meant it never hit the escaping case in > mctp_serial_tx_work(). > > Example Input: 01 00 08 c8 7e 80 02 > > Previous incorrect chunks from next_chunk_len(): > > 01 00 08 > c8 7e 80 > 02 > > With this fix: > > 01 00 08 c8 > 7e > 80 02 > > Cc: stable@vger.kernel.org > Fixes: a0c2ccd9b5ad ("mctp: Add MCTP-over-serial transport binding") > Signed-off-by: Matt Johnston <matt@codeconstruct.com.au> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com> > --- > drivers/net/mctp/mctp-serial.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c > index d7db11355909..82890e983847 100644 > --- a/drivers/net/mctp/mctp-serial.c > +++ b/drivers/net/mctp/mctp-serial.c > @@ -91,8 +91,8 @@ static int next_chunk_len(struct mctp_serial *dev) > * will be those non-escaped bytes, and does not include the escaped > * byte. > */ > - for (i = 1; i + dev->txpos + 1 < dev->txlen; i++) { > - if (needs_escape(dev->txbuf[dev->txpos + i + 1])) > + for (i = 1; i + dev->txpos < dev->txlen; i++) { > + if (needs_escape(dev->txbuf[dev->txpos + i])) > break; > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-29 7:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-27 2:07 [PATCH net 0/2] net: mctp-serial: Fix for missing tx escapes Matt Johnston 2024-08-27 2:07 ` [PATCH net 1/2] net: mctp-serial: Add kunit test for next_chunk_len() Matt Johnston 2024-08-27 23:13 ` kernel test robot 2024-08-29 7:30 ` kernel test robot 2024-08-27 2:07 ` [PATCH net 2/2] net: mctp-serial: Fix missing escapes on transmit Matt Johnston 2024-08-27 11:06 ` Larysa Zaremba
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).