[Devel] [PATCH] overlayfs: add dynamic path resolving in mount options

[Alexander Mikhalitsyn]

Thank you for your review.

I've sent a new patch version (2) where I've fixed all issues.

________________________________________
From: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
Sent: Friday, May 22, 2020 15:22
To: Alexander Mikhalitsyn; devel at openvz.org
Cc: Konstantin Khorenko
Subject: Re: [PATCH] overlayfs: add dynamic path resolving in mount options

>> +int print_paths_option(struct seq_file *m, const char *name, struct
>> ovl_fs *ofs)
>> +{
>> +    unsigned int order = ilog2(ofs->numlower) + 1;
>> +    char *res = (char*)__get_free_pages(GFP_TEMPORARY, order);
>> +    char *tmp = (char*)__get_free_page(GFP_TEMPORARY);
>> +    char *pathname;
>> +    int len;
>> +    int i;
>> +    char *shift;
>> +
>> +    if (!res)
>> +        return -ENOMEM;
>> +
>> +    if (!tmp)
>> +        return -ENOME > +
>> +    shift = res;
>> +    for (i = 0; i < ofs->numlower; i++) {
>> +        pathname = d_path(&ofs->lowerpaths[i], tmp, PAGE_SIZE);
>> +        len = PTR_ERR(pathname);
>> +        if (IS_ERR(pathname))
>> +            goto out;
>> +        len = tmp + PAGE_SIZE - 1 - pathname;
>> +        pathname[len] = 0;
>> +
>> +        *shift = ':';
>> +        memcpy(++shift, pathname, len);
>> +        shift += len;
>
> And all the same applies to this function too.
>
> More over with memcpy you don't controll overflow. You can:
>
> len = snprintf(shift, remaining_lenth, "%s", pathname);
> if (len >= remaining_lenth) {
>      /* overflow */
> }
> shift += len;
>
> I think "len = strlen(pathname)" looks better than "tmp + PAGE_SIZE - 1
> - pathname" though maybe a bit less effective. Or you can use snprintf
> instead of memcpy, it will also identify printed len.

upd: from personal talk with Alexander: maybe we don't even need high
order buffer allocation at all here and can print each lowerdir one by
one using just one page.

>
>> +    }
>> +    *shift = 0;
>> +
>> +    seq_show_option(m, name, res+1);
>> + out:
>> +    free_pages((unsigned long)res, order);
>> +    free_page((unsigned long)tmp);
>> +
>> +    return 0;
>> +}
>>
>

--
Best regards, Tikhomirov Pavel
Software Developer, Virtuozzo.