| Message ID | VI1PR06MB402928E3B0E4C887104BCE22D2B29@VI1PR06MB4029.eurprd06.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: tools: fix input string formatting | expand |
On Sat, Feb 06, 2021 at 10:57:04AM +0000, Aleksandar Gerasimovski wrote: > AG: to be sure we understand each other, you expect quotes to be sent to spi as well? That's expected by design behavior? > Is there any possibility to avoid sending them then? If you don't want quotes to be sent then don't send them - my expectation would be that if you're driving this from the shell then the shell would not be passing unescaped quotes through to the program. For example: $ echo "Hello, world!" Hello, world! Here echo only saw the unqouted string so that's what it displays. If you're not using a shell and starting this from another program then I'd assume there's some way to generate unquoted arguments in whatever you're using. > > This appears to be requiring that anything passed into unescape() be a number which isn't something we'd obviously want? I'd expect the function to unescape things, not to do other random validation which may or may not be appropriate in context. > AG: so by design is expected that everything is accepted here, e.g \"1234qwert\\xde\\xad\"? If yes than I understood this tool wrongly. Yes, that's my expecation - it's just processing escape sequences and passing everything else through.
Ok, thanks for clarification. You can forget this patch! -----Original Message----- From: Mark Brown <broonie@kernel.org> Sent: Montag, 8. Februar 2021 11:09 To: Aleksandar Gerasimovski <aleksandar.gerasimovski@hitachi-powergrids.com> Cc: linux-spi@vger.kernel.org Subject: Re: [PATCH] spi: tools: fix input string formatting On Sat, Feb 06, 2021 at 10:57:04AM +0000, Aleksandar Gerasimovski wrote: > AG: to be sure we understand each other, you expect quotes to be sent to spi as well? That's expected by design behavior? > Is there any possibility to avoid sending them then? If you don't want quotes to be sent then don't send them - my expectation would be that if you're driving this from the shell then the shell would not be passing unescaped quotes through to the program. For example: $ echo "Hello, world!" Hello, world! Here echo only saw the unqouted string so that's what it displays. If you're not using a shell and starting this from another program then I'd assume there's some way to generate unquoted arguments in whatever you're using. > > This appears to be requiring that anything passed into unescape() be a number which isn't something we'd obviously want? I'd expect the function to unescape things, not to do other random validation which may or may not be appropriate in context. > AG: so by design is expected that everything is accepted here, e.g \"1234qwert\\xde\\xad\"? If yes than I understood this tool wrongly. Yes, that's my expecation - it's just processing escape sequences and passing everything else through.
diff --git a/tools/spi/spidev_test.c b/tools/spi/spidev_test.c index 83844f8..7c36677 100644 --- a/tools/spi/spidev_test.c +++ b/tools/spi/spidev_test.c @@ -89,7 +89,7 @@ static void hex_dump(const void *src, size_t length, size_t line_size, /* * Unescape - process hexadecimal escape character - * converts shell input "\x23" -> 0x23 + * converts shell input "\\x23" -> 0x23 */ static int unescape(char *_dst, char *_src, size_t len) { @@ -100,6 +100,10 @@ static int unescape(char *_dst, char *_src, size_t len) unsigned int ch; while (*src) { + if (*src == '"') { + src++; + continue; + } if (*src == '\\' && *(src+1) == 'x') { match = sscanf(src + 2, "%2x", &ch); if (!match) @@ -108,6 +112,9 @@ static int unescape(char *_dst, char *_src, size_t len) src += 4; *dst++ = (unsigned char)ch; } else { + match = sscanf(src, "%2d", &ch); + if (!match) + pabort("malformed input string"); *dst++ = *src++; } ret++;
The actual unescape implementation has two bugs: 1. quotation marks from the input string are not removed and are sent to the spidev, e.g: input string: \"\\xFE\\x01\" will be sent to the spidev as 0x22 0xfe 0x01 0x22 2. there is not format check for decimal input strings First bug makes spidev_test unusable when strict spi sequence is needed, second bug is not nice to have it in. This patch improves unescape function and fixes above listed bugs. Signed-off-by: Aleksandar Gerasimovski <aleksandar.gerasimovski@hitachi-powergrids.com> --- tools/spi/spidev_test.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)