netdev - Re: [PATCH bpf-next v3 4/9] bpf: Implement cgroup sockaddr hooks for unix sockets

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] [day] [month] [year] [list]

Message-ID: <6ad30137-c7d7-884b-c19e-e16288984d57@linux.dev>
Date: Tue, 5 Sep 2023 14:38:26 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Daan De Meyer <daan.j.demeyer@...il.com>
Cc: kernel-team@...a.com, netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 4/9] bpf: Implement cgroup sockaddr hooks for
 unix sockets

On 9/5/23 12:02 PM, Martin KaFai Lau wrote:
>> @@ -1766,14 +1787,37 @@ static int unix_getname(struct socket *sock, struct 
>> sockaddr *uaddr, int peer)
>>       if (!addr) {
>>           sunaddr->sun_family = AF_UNIX;
>>           sunaddr->sun_path[0] = 0;
>> -        err = offsetof(struct sockaddr_un, sun_path);
>> +        addr_len = offsetof(struct sockaddr_un, sun_path);
>>       } else {
>> -        err = addr->len;
>> +        addr_len = addr->len;
>>           memcpy(sunaddr, addr->name, addr->len);
>>       }
>> +
>> +    if (peer && cgroup_bpf_enabled(CGROUP_UNIX_GETPEERNAME)) {
>> +        err = BPF_CGROUP_RUN_SA_PROG(sk, uaddr, &addr_len,
>> +                         CGROUP_UNIX_GETPEERNAME);
>> +        if (err)
> 
> UNIX_GETPEERNAME can only have return value 1 (OK), so no need to do err check 
> here.
> 
>> +            goto out;
>> +
>> +        err = unix_validate_addr(sunaddr, addr_len);
> 
> Since the kfunc is specific to the unix address, how about doing the 
> unix_validate_addr check in the kfunc itself?

When reading patch 3 again, the kfunc has already checked the addrlen with the 
UNIX_PATH_MAX. It should be as good as unix_validate_addr() check considering 
the kfunc can only change the sunaddr->sun_path?