| Message ID | 20240424081957.34326-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | livepatch: minor bug fixes and improvements | expand |
On Wed, Apr 24, 2024 at 10:19:54AM +0200, 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. Use strcmp instead which > doesn't truncate. Otherwise in order to keep using strncmp we would need to > also check strings are of the same length before doing the comparison. > > Fixes: 05bb8afedede ('xen-xsplice: Tool to manipulate xsplice payloads') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Acked-by: Anthony PERARD <anthony.perard@citrix.com> Thanks,
On Wed, Apr 24, 2024 at 9:20 AM Roger Pau Monne <roger.pau@citrix.com> 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. Use strcmp instead which > doesn't truncate. Otherwise in order to keep using strncmp we would need to > also check strings are of the same length before doing the comparison. > I had previously assumed that this was intentional as a way of allowing abbreviated syntax. Regardless of the original intent, I'm OK with this change. Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c index 5bf9d9a32b65..2c4f69e596fa 100644 --- a/tools/misc/xen-livepatch.c +++ b/tools/misc/xen-livepatch.c @@ -572,13 +572,13 @@ 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 (!strcmp(main_options[i].name, argv[1])) 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 (!strcmp(action_options[j].name, argv[1])) 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. Use strcmp instead which doesn't truncate. Otherwise in order to keep using strncmp we would need to also check strings are of the same length before doing the comparison. Fixes: 05bb8afedede ('xen-xsplice: Tool to manipulate xsplice payloads') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v3: - Use strcmp. Changes since v2: - New in this version. --- tools/misc/xen-livepatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)