| Message ID | 20240423131249.29825-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | livepatch: minor bug fixes and improvements | expand |
On 23.04.2024 15:12, Roger Pau Monne wrote: > It's incorrect to restrict strncmp to the length of the command line input > parameter, as then a user passing a rune like: > > % xen-livepatch up foo.livepatch > > Would match against the "upload" command, because the string comparison has > been truncated to the length of the input argument. Instead the truncation > should be done based on the length of the command name stored in the internal > array of actions. But then "xen-livepatch upload-or-not foo.livepatch" would still wrongly match. Why strncmp() at all, rather than strcmp()? Jan > Fixes: 05bb8afedede ('xen-xsplice: Tool to manipulate xsplice payloads') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Changes since v2: > - New in this version. > --- > tools/misc/xen-livepatch.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c > index 5bf9d9a32b65..a246e5dfd38e 100644 > --- a/tools/misc/xen-livepatch.c > +++ b/tools/misc/xen-livepatch.c > @@ -572,13 +572,15 @@ int main(int argc, char *argv[]) > return 0; > } > for ( i = 0; i < ARRAY_SIZE(main_options); i++ ) > - if (!strncmp(main_options[i].name, argv[1], strlen(argv[1]))) > + if (!strncmp(main_options[i].name, argv[1], > + strlen(main_options[i].name))) > break; > > if ( i == ARRAY_SIZE(main_options) ) > { > for ( j = 0; j < ARRAY_SIZE(action_options); j++ ) > - if (!strncmp(action_options[j].name, argv[1], strlen(argv[1]))) > + if (!strncmp(action_options[j].name, argv[1], > + strlen(action_options[j].name))) > break; > > if ( j == ARRAY_SIZE(action_options) )
On Tue, Apr 23, 2024 at 03:33:36PM +0200, Jan Beulich wrote: > On 23.04.2024 15:12, Roger Pau Monne wrote: > > It's incorrect to restrict strncmp to the length of the command line input > > parameter, as then a user passing a rune like: > > > > % xen-livepatch up foo.livepatch > > > > Would match against the "upload" command, because the string comparison has > > been truncated to the length of the input argument. Instead the truncation > > should be done based on the length of the command name stored in the internal > > array of actions. > > But then "xen-livepatch upload-or-not foo.livepatch" would still wrongly > match. Why strncmp() at all, rather than strcmp()? Bah, indeed, how dumb of me. Will switch to strcmp in the next version. Thanks, Roger.-
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c index 5bf9d9a32b65..a246e5dfd38e 100644 --- a/tools/misc/xen-livepatch.c +++ b/tools/misc/xen-livepatch.c @@ -572,13 +572,15 @@ int main(int argc, char *argv[]) return 0; } for ( i = 0; i < ARRAY_SIZE(main_options); i++ ) - if (!strncmp(main_options[i].name, argv[1], strlen(argv[1]))) + if (!strncmp(main_options[i].name, argv[1], + strlen(main_options[i].name))) break; if ( i == ARRAY_SIZE(main_options) ) { for ( j = 0; j < ARRAY_SIZE(action_options); j++ ) - if (!strncmp(action_options[j].name, argv[1], strlen(argv[1]))) + if (!strncmp(action_options[j].name, argv[1], + strlen(action_options[j].name))) break; if ( j == ARRAY_SIZE(action_options) )
It's incorrect to restrict strncmp to the length of the command line input parameter, as then a user passing a rune like: % xen-livepatch up foo.livepatch Would match against the "upload" command, because the string comparison has been truncated to the length of the input argument. Instead the truncation should be done based on the length of the command name stored in the internal array of actions. Fixes: 05bb8afedede ('xen-xsplice: Tool to manipulate xsplice payloads') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v2: - New in this version. --- tools/misc/xen-livepatch.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)