Re: [PATCH bpf-next v2 1/4] bpf: Don't EFAULT for {g,s}setsockopt with wrong optlen

[Martin KaFai Lau <martin.lau@linux.dev>]

On 5/1/23 9:55 AM, Stanislav Fomichev wrote:
> On Sun, Apr 30, 2023 at 10:52 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>
>> On 4/27/23 1:04 PM, Stanislav Fomichev wrote:
>>> @@ -1881,8 +1886,10 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
>>>                .optname = optname,
>>>                .current_task = current,
>>>        };
>>> +     int orig_optlen;
>>>        int ret;
>>>
>>> +     orig_optlen = max_optlen;
>>
>> For getsockopt, when the kernel's getsockopt finished successfully (the
>> following 'if (!retval)' case), how about also setting orig_optlen to the kernel
>> returned 'optlen'. For example, the user's orig_optlen is 8096 and the kernel
>> returned optlen is 1024. If the bpf prog still sets the ctx.optlen to something
>>   > PAGE_SIZE, -EFAULT will be returned.
> 
> Wouldn't it defeat the purpose? Or am I missing something?
> 
> ctx.optlen would still be 8096, not 1024, right (regardless of what
> the kernel returns)?
> So it would trigger EFAULT case which we try to avoid.

My understanding is the ctx.optlen should be 1024 after the 'if (!retval)' 
statement.

The 'int __user *optlen' arg has the kernel returned optlen (1024). The 'int 
max_optlen' arg has the original user's optlen (8096).

int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
				       int optname, char __user *optval,
				       int __user *optlen /* 1024 */,
				       int max_optlen /* 8096 */,
				       int retval)
{
	/* ... */

	orig_optlen = max_optlen; /* orig_optlen == 8096 */
	ctx.optlen = max_optlen;  /* ctx.optlen == 8096 */

	
	if (!retval) {
		/* If kernel getsockopt finished successfully,
		 * copy whatever was returned to the user back
		 * into our temporary buffer. Set optlen to the
		 * one that kernel returned as well to let
		 * BPF programs inspect the value.
		 */

		if (get_user(ctx.optlen, optlen)) {
			ret = -EFAULT;
			goto out;
		}

		/* ctx.optlen == 1024 */

		orig_optlen = ctx.optlen;
	}

	/* ... */
}