On Mon, May 1, 2023 at 11:58 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 5/1/23 9:55 AM, Stanislav Fomichev wrote:
> > On Sun, Apr 30, 2023 at 10:52 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> 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.
Ah, you're right, thanks! Will add your suggestion.
> 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;
> }
>
> /* ... */
> }
