| Message ID | 20240904104824.1844082-18-ivanov.mikhail1@huawei-partners.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | Support socket access-control | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch, async |
On Wed, Sep 04, 2024 at 06:48:22PM +0800, Mikhail Ivanov wrote: > Add str2num() helper and replace atoi() with it. atoi() does not provide > overflow checks, checks of invalid characters in a string and it is > recommended to use strtol-like functions (Cf. atoi() manpage). > > Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> > --- > samples/landlock/sandboxer.c | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c > index e8223c3e781a..d4dba9e4ce89 100644 > --- a/samples/landlock/sandboxer.c > +++ b/samples/landlock/sandboxer.c > @@ -150,6 +150,26 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd, > return ret; > } > > +static int str2num(const char *numstr, unsigned long long *num_dst) > +{ > + char *endptr = NULL; > + int err = 1; > + unsigned long long num; > + > + errno = 0; > + num = strtoull(numstr, &endptr, 0); > + if (errno != 0) > + goto out; > + > + if (*endptr != '\0') > + goto out; > + > + *num_dst = num; > + err = 0; > +out: > + return err; > +} I believe if numstr is the empty string, str2num would return success and set num_dst to 0, which looks unintentional to me. Do we not have a better helper for this that we can link from here? > + > static int populate_ruleset_net(const char *const env_var, const int ruleset_fd, > const __u64 allowed_access) > { > @@ -168,7 +188,12 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd, > > env_port_name_next = env_port_name; > while ((strport = strsep(&env_port_name_next, ENV_DELIMITER))) { > - net_port.port = atoi(strport); > + if (str2num(strport, &net_port.port)) { > + fprintf(stderr, > + "Failed to convert \"%s\" into a number\n", > + strport); > + goto out_free_name; > + } > if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT, > &net_port, 0)) { > fprintf(stderr, > -- > 2.34.1 >
On 9/27/2024 6:12 PM, Günther Noack wrote: > On Wed, Sep 04, 2024 at 06:48:22PM +0800, Mikhail Ivanov wrote: >> Add str2num() helper and replace atoi() with it. atoi() does not provide >> overflow checks, checks of invalid characters in a string and it is >> recommended to use strtol-like functions (Cf. atoi() manpage). >> >> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> >> --- >> samples/landlock/sandboxer.c | 27 ++++++++++++++++++++++++++- >> 1 file changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c >> index e8223c3e781a..d4dba9e4ce89 100644 >> --- a/samples/landlock/sandboxer.c >> +++ b/samples/landlock/sandboxer.c >> @@ -150,6 +150,26 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd, >> return ret; >> } >> >> +static int str2num(const char *numstr, unsigned long long *num_dst) >> +{ >> + char *endptr = NULL; >> + int err = 1; >> + unsigned long long num; >> + >> + errno = 0; >> + num = strtoull(numstr, &endptr, 0); >> + if (errno != 0) >> + goto out; >> + >> + if (*endptr != '\0') >> + goto out; >> + >> + *num_dst = num; >> + err = 0; >> +out: >> + return err; >> +} > > I believe if numstr is the empty string, str2num would return success and set > num_dst to 0, which looks unintentional to me. Yeap.. I'll fix this > > Do we not have a better helper for this that we can link from here? I've checked how such convertion is performed in selftests by another subsystems and it seems that most common practise is to implement static helper or inline convertion in the needed place (e.g. safe_int() in selftests/net/af_unix/scm_pidfd.c). > >> + >> static int populate_ruleset_net(const char *const env_var, const int ruleset_fd, >> const __u64 allowed_access) >> { >> @@ -168,7 +188,12 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd, >> >> env_port_name_next = env_port_name; >> while ((strport = strsep(&env_port_name_next, ENV_DELIMITER))) { >> - net_port.port = atoi(strport); >> + if (str2num(strport, &net_port.port)) { >> + fprintf(stderr, >> + "Failed to convert \"%s\" into a number\n", >> + strport); >> + goto out_free_name; >> + } >> if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT, >> &net_port, 0)) { >> fprintf(stderr, >> -- >> 2.34.1 >>
diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c index e8223c3e781a..d4dba9e4ce89 100644 --- a/samples/landlock/sandboxer.c +++ b/samples/landlock/sandboxer.c @@ -150,6 +150,26 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd, return ret; } +static int str2num(const char *numstr, unsigned long long *num_dst) +{ + char *endptr = NULL; + int err = 1; + unsigned long long num; + + errno = 0; + num = strtoull(numstr, &endptr, 0); + if (errno != 0) + goto out; + + if (*endptr != '\0') + goto out; + + *num_dst = num; + err = 0; +out: + return err; +} + static int populate_ruleset_net(const char *const env_var, const int ruleset_fd, const __u64 allowed_access) { @@ -168,7 +188,12 @@ static int populate_ruleset_net(const char *const env_var, const int ruleset_fd, env_port_name_next = env_port_name; while ((strport = strsep(&env_port_name_next, ENV_DELIMITER))) { - net_port.port = atoi(strport); + if (str2num(strport, &net_port.port)) { + fprintf(stderr, + "Failed to convert \"%s\" into a number\n", + strport); + goto out_free_name; + } if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_NET_PORT, &net_port, 0)) { fprintf(stderr,
Add str2num() helper and replace atoi() with it. atoi() does not provide overflow checks, checks of invalid characters in a string and it is recommended to use strtol-like functions (Cf. atoi() manpage). Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com> --- samples/landlock/sandboxer.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)