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@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxx> thanks, Mimi > + continue; > + } > > INIT_LIST_HEAD(&arch_policy_entry[i].list); > result = ima_parse_rule(rule, &arch_policy_entry[i]); >
