Messages in this thread | | | Subject | Re: [PATCH 06/20] ima: Manual replacement of the deprecated strlcpy() with return values | From | Mimi Zohar <> | Date | Tue, 02 Mar 2021 08:29:58 -0500 |
| |
On Mon, 2021-02-22 at 16:12 +0100, Romain Perier wrote: > The strlcpy() reads the entire source buffer first, it is dangerous if > the source buffer lenght is unbounded or possibility non NULL-terminated.
As other's have pointed out, "lenght" -> length.
> It can lead to linear read overflows, crashes, etc... > > As recommended in the deprecated interfaces [1], it should be replaced > by strscpy. > > This commit replaces all calls to strlcpy that handle the return values > by the corresponding strscpy calls with new handling of the return > values (as it is quite different between the two functions). > > [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy > > Signed-off-by: Romain Perier <romain.perier@gmail.com> > --- > security/integrity/ima/ima_policy.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 9b45d064a87d..1a905b8b064f 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -790,8 +790,14 @@ static int __init ima_init_arch_policy(void) > for (rules = arch_rules, i = 0; *rules != NULL; rules++) { > char rule[255]; > int result; > + ssize_t len; > > - result = strlcpy(rule, *rules, sizeof(rule)); > + len = strscpy(rule, *rules, sizeof(rule)); > + if (len == -E2BIG) { > + pr_warn("Internal copy of architecture policy rule '%s' " > + "failed. Skipping.\n", *rules);
"arch_rules" is an array of hard coded strings. The generic reason for replacing strlcpy with strscpy doesn't seem applicable; however, the additonal warning is appropriate.
(User-visible strings are not bound to the 80 column length. Breaking up the line like this is fine, but unnecessary.)
Acked-by: Mimi Zohar <zohar@linux.ibm.com>
thanks,
Mimi
> + continue; > + } > > INIT_LIST_HEAD(&arch_policy_entry[i].list); > result = ima_parse_rule(rule, &arch_policy_entry[i]); >
| |