netdev - Re: [PATCH bpf-next v12 1/2] bpf: add new helper get_file_path for mapping a file descriptor to a pathname

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fecf759e-2941-3920-82d5-45a556f4dd1d@fb.com>
Date:   Tue, 17 Dec 2019 06:33:12 +0000
From:   Yonghong Song <yhs@...com>
To:     Wenbo Zhang <ethercflow@...il.com>
CC:     "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "ast@...nel.org" <ast@...nel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "andrii.nakryiko@...il.com" <andrii.nakryiko@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next v12 1/2] bpf: add new helper get_file_path for
 mapping a file descriptor to a pathname



On 12/16/19 10:26 PM, Wenbo Zhang wrote:
>>> + *           - a regular full path (include mountable fs eg: /proc, /sys)
>>> + *           - a regular full path with "(deleted)" at the end.
> 
>> Let us say with " (deleted)" is appended to be consistent with comments
>> in d_path() and is more clear to user what the format will looks like.
> 
> Thank you, I'll fix this.
> 
>>> +     ret = strlen(p);
>>> +     memmove(dst, p, ret);
>>> +     dst[ret++] = '\0';
> 
>> nit: you could do memmove(dst, p, ret + 1)?
> 
> I did with `dst[ret++]='\0';`  to return value length including
> trailing '\0'. as you mentioned below:
> 
>>> +     fput(f);
>>> +     return ret;
> 
>> The description says the return value length including trailing '\0'.
>> The above 'ret' does not include trailing '\0'.
> 
> It seems `[ret++]` not very clear to read and '\0' can be done by
> `memmove`. I think I'll refactor to
> 
> ```
> ret = strlen(p) + 1;
> memmove(dst, p, ret);
> fput(f);
> return ret;
> ```
> 
> Is this better?

Ah, I missed ret++ in dst[ret++]. Indeed the above code is better.