diff mbox series

[bpf-next,v3,05/15] bpf: add bpf_skc_to_tcp6_sock() helper

Message ID 20200623003631.3073864-1-yhs@fb.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series implement bpf iterator for tcp and udp sockets | expand

Commit Message

Yonghong Song June 23, 2020, 12:36 a.m. UTC 
The helper is used in tracing programs to cast a socket
pointer to a tcp6_sock pointer.
The return value could be NULL if the casting is illegal.

A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
so the verifier is able to deduce proper return types for the helper.

Different from the previous BTF_ID based helpers,
the bpf_skc_to_tcp6_sock() argument can be several possible
btf_ids. More specifically, all possible socket data structures
with sock_common appearing in the first in the memory layout.
This patch only added socket types related to tcp and udp.

All possible argument btf_id and return value btf_id
for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
cached. In the future, it is even possible to precompute
these btf_id's at kernel build time.

Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/bpf.h            | 12 +++++
 include/uapi/linux/bpf.h       |  9 +++-
 kernel/bpf/btf.c               |  1 +
 kernel/bpf/verifier.c          | 43 +++++++++++++-----
 kernel/trace/bpf_trace.c       |  2 +
 net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
 scripts/bpf_helpers_doc.py     |  2 +
 tools/include/uapi/linux/bpf.h |  9 +++-
 8 files changed, 146 insertions(+), 12 deletions(-)

Comments

kernel test robot June 23, 2020, 5:46 a.m. UTC | #1 
Hi Yonghong,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Yonghong-Song/implement-bpf-iterator-for-tcp-and-udp-sockets/20200623-090149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: parisc-defconfig (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   hppa-linux-ld: net/core/filter.o: in function `init_btf_sock_ids':
>> (.text+0xe878): undefined reference to `btf_find_by_name_kind'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot June 23, 2020, 5:53 a.m. UTC | #2 
Hi Yonghong,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/0day-ci/linux/commits/Yonghong-Song/implement-bpf-iterator-for-tcp-and-udp-sockets/20200623-090149
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: arc-defconfig (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arc-elf-ld: net/core/filter.o: in function `init_btf_sock_ids':
   filter.c:(.text+0xaf22): undefined reference to `btf_find_by_name_kind'
>> arc-elf-ld: filter.c:(.text+0xaf22): undefined reference to `btf_find_by_name_kind'

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Andrii Nakryiko June 23, 2020, 6:39 a.m. UTC | #3 
On Mon, Jun 22, 2020 at 5:38 PM Yonghong Song <yhs@fb.com> wrote:
>
> The helper is used in tracing programs to cast a socket
> pointer to a tcp6_sock pointer.
> The return value could be NULL if the casting is illegal.
>
> A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
> so the verifier is able to deduce proper return types for the helper.
>
> Different from the previous BTF_ID based helpers,
> the bpf_skc_to_tcp6_sock() argument can be several possible
> btf_ids. More specifically, all possible socket data structures
> with sock_common appearing in the first in the memory layout.
> This patch only added socket types related to tcp and udp.
>
> All possible argument btf_id and return value btf_id
> for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
> cached. In the future, it is even possible to precompute
> these btf_id's at kernel build time.
>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---

Looks good to me as is, but see a few suggestions, they will probably
save me time at some point as well :)

Acked-by: Andrii Nakryiko <andriin@fb.com>


>  include/linux/bpf.h            | 12 +++++
>  include/uapi/linux/bpf.h       |  9 +++-
>  kernel/bpf/btf.c               |  1 +
>  kernel/bpf/verifier.c          | 43 +++++++++++++-----
>  kernel/trace/bpf_trace.c       |  2 +
>  net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
>  scripts/bpf_helpers_doc.py     |  2 +
>  tools/include/uapi/linux/bpf.h |  9 +++-
>  8 files changed, 146 insertions(+), 12 deletions(-)
>

[...]

> @@ -4815,6 +4826,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>                 regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
>                 regs[BPF_REG_0].id = ++env->id_gen;
>                 regs[BPF_REG_0].mem_size = meta.mem_size;
> +       } else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
> +               int ret_btf_id;
> +
> +               mark_reg_known_zero(env, regs, BPF_REG_0);
> +               regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
> +               ret_btf_id = *fn->ret_btf_id;

might be a good idea to check fb->ret_btf_id for NULL and print a
warning + return -EFAULT. It's not supposed to happen on properly
configured kernel, but during development this will save a bunch of
time and frustration for next person trying to add something with
RET_PTR_TO_BTF_ID_OR_NULL.

> +               if (ret_btf_id == 0) {

This also has to be struct/union (after typedef/mods stripping, of
course). Or are there other cases?

> +                       verbose(env, "invalid return type %d of func %s#%d\n",
> +                               fn->ret_type, func_id_name(func_id), func_id);
> +                       return -EINVAL;
> +               }
> +               regs[BPF_REG_0].btf_id = ret_btf_id;
>         } else {
>                 verbose(env, "unknown return type %d of func %s#%d\n",
>                         fn->ret_type, func_id_name(func_id), func_id);

[...]

> +void init_btf_sock_ids(struct btf *btf)
> +{
> +       int i, btf_id;
> +
> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
> +               btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
> +                                              BTF_KIND_STRUCT);
> +               if (btf_id > 0)
> +                       btf_sock_ids[i] = btf_id;
> +       }
> +}

This will hopefully go away with Jiri's work on static BTF IDs, right?
So looking forward to that :)

> +
> +static bool check_arg_btf_id(u32 btf_id, u32 arg)
> +{
> +       int i;
> +
> +       /* only one argument, no need to check arg */
> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
> +               if (btf_sock_ids[i] == btf_id)
> +                       return true;
> +       return false;
> +}
> +

[...]
Yonghong Song June 23, 2020, 2:52 p.m. UTC | #4 
On 6/22/20 11:39 PM, Andrii Nakryiko wrote:
> On Mon, Jun 22, 2020 at 5:38 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> The helper is used in tracing programs to cast a socket
>> pointer to a tcp6_sock pointer.
>> The return value could be NULL if the casting is illegal.
>>
>> A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
>> so the verifier is able to deduce proper return types for the helper.
>>
>> Different from the previous BTF_ID based helpers,
>> the bpf_skc_to_tcp6_sock() argument can be several possible
>> btf_ids. More specifically, all possible socket data structures
>> with sock_common appearing in the first in the memory layout.
>> This patch only added socket types related to tcp and udp.
>>
>> All possible argument btf_id and return value btf_id
>> for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
>> cached. In the future, it is even possible to precompute
>> these btf_id's at kernel build time.
>>
>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
> 
> Looks good to me as is, but see a few suggestions, they will probably
> save me time at some point as well :)
> 
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> 
> 
>>   include/linux/bpf.h            | 12 +++++
>>   include/uapi/linux/bpf.h       |  9 +++-
>>   kernel/bpf/btf.c               |  1 +
>>   kernel/bpf/verifier.c          | 43 +++++++++++++-----
>>   kernel/trace/bpf_trace.c       |  2 +
>>   net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
>>   scripts/bpf_helpers_doc.py     |  2 +
>>   tools/include/uapi/linux/bpf.h |  9 +++-
>>   8 files changed, 146 insertions(+), 12 deletions(-)
>>
> 
> [...]
> 
>> @@ -4815,6 +4826,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>>                  regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
>>                  regs[BPF_REG_0].id = ++env->id_gen;
>>                  regs[BPF_REG_0].mem_size = meta.mem_size;
>> +       } else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
>> +               int ret_btf_id;
>> +
>> +               mark_reg_known_zero(env, regs, BPF_REG_0);
>> +               regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
>> +               ret_btf_id = *fn->ret_btf_id;
> 
> might be a good idea to check fb->ret_btf_id for NULL and print a
> warning + return -EFAULT. It's not supposed to happen on properly
> configured kernel, but during development this will save a bunch of
> time and frustration for next person trying to add something with
> RET_PTR_TO_BTF_ID_OR_NULL.

I would like prefer to delay this with current code. Otherwise,
it gives an impression fn->ret_btf_id might be NULL and it is
actually never NULL. We can add NULL check if the future change
requires it. I am not sure what the future change could be,
but you need some way to get the return btf_id, the above is
one of them.

> 
>> +               if (ret_btf_id == 0) {
> 
> This also has to be struct/union (after typedef/mods stripping, of
> course). Or are there other cases?

This is an "int". The func_proto difinition is below:
int *ret_btf_id; /* return value btf_id */

> 
>> +                       verbose(env, "invalid return type %d of func %s#%d\n",
>> +                               fn->ret_type, func_id_name(func_id), func_id);
>> +                       return -EINVAL;
>> +               }
>> +               regs[BPF_REG_0].btf_id = ret_btf_id;
>>          } else {
>>                  verbose(env, "unknown return type %d of func %s#%d\n",
>>                          fn->ret_type, func_id_name(func_id), func_id);
> 
> [...]
> 
>> +void init_btf_sock_ids(struct btf *btf)
>> +{
>> +       int i, btf_id;
>> +
>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
>> +               btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
>> +                                              BTF_KIND_STRUCT);
>> +               if (btf_id > 0)
>> +                       btf_sock_ids[i] = btf_id;
>> +       }
>> +}
> 
> This will hopefully go away with Jiri's work on static BTF IDs, right?
> So looking forward to that :)

Yes. That's the plan.

> 
>> +
>> +static bool check_arg_btf_id(u32 btf_id, u32 arg)
>> +{
>> +       int i;
>> +
>> +       /* only one argument, no need to check arg */
>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
>> +               if (btf_sock_ids[i] == btf_id)
>> +                       return true;
>> +       return false;
>> +}
>> +
> 
> [...]
>
Andrii Nakryiko June 23, 2020, 6:23 p.m. UTC | #5 
On Tue, Jun 23, 2020 at 7:52 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/22/20 11:39 PM, Andrii Nakryiko wrote:
> > On Mon, Jun 22, 2020 at 5:38 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> The helper is used in tracing programs to cast a socket
> >> pointer to a tcp6_sock pointer.
> >> The return value could be NULL if the casting is illegal.
> >>
> >> A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
> >> so the verifier is able to deduce proper return types for the helper.
> >>
> >> Different from the previous BTF_ID based helpers,
> >> the bpf_skc_to_tcp6_sock() argument can be several possible
> >> btf_ids. More specifically, all possible socket data structures
> >> with sock_common appearing in the first in the memory layout.
> >> This patch only added socket types related to tcp and udp.
> >>
> >> All possible argument btf_id and return value btf_id
> >> for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
> >> cached. In the future, it is even possible to precompute
> >> these btf_id's at kernel build time.
> >>
> >> Acked-by: Martin KaFai Lau <kafai@fb.com>
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >
> > Looks good to me as is, but see a few suggestions, they will probably
> > save me time at some point as well :)
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> >
> >
> >>   include/linux/bpf.h            | 12 +++++
> >>   include/uapi/linux/bpf.h       |  9 +++-
> >>   kernel/bpf/btf.c               |  1 +
> >>   kernel/bpf/verifier.c          | 43 +++++++++++++-----
> >>   kernel/trace/bpf_trace.c       |  2 +
> >>   net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
> >>   scripts/bpf_helpers_doc.py     |  2 +
> >>   tools/include/uapi/linux/bpf.h |  9 +++-
> >>   8 files changed, 146 insertions(+), 12 deletions(-)
> >>
> >
> > [...]
> >
> >> @@ -4815,6 +4826,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
> >>                  regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
> >>                  regs[BPF_REG_0].id = ++env->id_gen;
> >>                  regs[BPF_REG_0].mem_size = meta.mem_size;
> >> +       } else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
> >> +               int ret_btf_id;
> >> +
> >> +               mark_reg_known_zero(env, regs, BPF_REG_0);
> >> +               regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
> >> +               ret_btf_id = *fn->ret_btf_id;
> >
> > might be a good idea to check fb->ret_btf_id for NULL and print a
> > warning + return -EFAULT. It's not supposed to happen on properly
> > configured kernel, but during development this will save a bunch of
> > time and frustration for next person trying to add something with
> > RET_PTR_TO_BTF_ID_OR_NULL.
>
> I would like prefer to delay this with current code. Otherwise,
> it gives an impression fn->ret_btf_id might be NULL and it is
> actually never NULL. We can add NULL check if the future change
> requires it. I am not sure what the future change could be,
> but you need some way to get the return btf_id, the above is
> one of them.

It's not **supposed** to be NULL, same as a bunch of other invariants
about BPF helper proto definitions, but verifier does check sanity for
such cases, instead of crashing. But up to you. I'm pretty sure
someone will trip up on this.

>
> >
> >> +               if (ret_btf_id == 0) {
> >
> > This also has to be struct/union (after typedef/mods stripping, of
> > course). Or are there other cases?
>
> This is an "int". The func_proto difinition is below:
> int *ret_btf_id; /* return value btf_id */

I meant the BTF type itself that this btf_id points to. Is there any
use case where this won't be a pointer to struct/union and instead
something like a pointer to an int?

>
> >
> >> +                       verbose(env, "invalid return type %d of func %s#%d\n",
> >> +                               fn->ret_type, func_id_name(func_id), func_id);
> >> +                       return -EINVAL;
> >> +               }
> >> +               regs[BPF_REG_0].btf_id = ret_btf_id;
> >>          } else {
> >>                  verbose(env, "unknown return type %d of func %s#%d\n",
> >>                          fn->ret_type, func_id_name(func_id), func_id);
> >
> > [...]
> >
> >> +void init_btf_sock_ids(struct btf *btf)
> >> +{
> >> +       int i, btf_id;
> >> +
> >> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
> >> +               btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
> >> +                                              BTF_KIND_STRUCT);
> >> +               if (btf_id > 0)
> >> +                       btf_sock_ids[i] = btf_id;
> >> +       }
> >> +}
> >
> > This will hopefully go away with Jiri's work on static BTF IDs, right?
> > So looking forward to that :)
>
> Yes. That's the plan.
>
> >
> >> +
> >> +static bool check_arg_btf_id(u32 btf_id, u32 arg)
> >> +{
> >> +       int i;
> >> +
> >> +       /* only one argument, no need to check arg */
> >> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
> >> +               if (btf_sock_ids[i] == btf_id)
> >> +                       return true;
> >> +       return false;
> >> +}
> >> +
> >
> > [...]
> >
Yonghong Song June 23, 2020, 7:45 p.m. UTC | #6 
On 6/23/20 11:23 AM, Andrii Nakryiko wrote:
> On Tue, Jun 23, 2020 at 7:52 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 6/22/20 11:39 PM, Andrii Nakryiko wrote:
>>> On Mon, Jun 22, 2020 at 5:38 PM Yonghong Song <yhs@fb.com> wrote:
>>>>
>>>> The helper is used in tracing programs to cast a socket
>>>> pointer to a tcp6_sock pointer.
>>>> The return value could be NULL if the casting is illegal.
>>>>
>>>> A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
>>>> so the verifier is able to deduce proper return types for the helper.
>>>>
>>>> Different from the previous BTF_ID based helpers,
>>>> the bpf_skc_to_tcp6_sock() argument can be several possible
>>>> btf_ids. More specifically, all possible socket data structures
>>>> with sock_common appearing in the first in the memory layout.
>>>> This patch only added socket types related to tcp and udp.
>>>>
>>>> All possible argument btf_id and return value btf_id
>>>> for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
>>>> cached. In the future, it is even possible to precompute
>>>> these btf_id's at kernel build time.
>>>>
>>>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>>> ---
>>>
>>> Looks good to me as is, but see a few suggestions, they will probably
>>> save me time at some point as well :)
>>>
>>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>>>
>>>
>>>>    include/linux/bpf.h            | 12 +++++
>>>>    include/uapi/linux/bpf.h       |  9 +++-
>>>>    kernel/bpf/btf.c               |  1 +
>>>>    kernel/bpf/verifier.c          | 43 +++++++++++++-----
>>>>    kernel/trace/bpf_trace.c       |  2 +
>>>>    net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
>>>>    scripts/bpf_helpers_doc.py     |  2 +
>>>>    tools/include/uapi/linux/bpf.h |  9 +++-
>>>>    8 files changed, 146 insertions(+), 12 deletions(-)
>>>>
>>>
>>> [...]
>>>
>>>> @@ -4815,6 +4826,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>>>>                   regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
>>>>                   regs[BPF_REG_0].id = ++env->id_gen;
>>>>                   regs[BPF_REG_0].mem_size = meta.mem_size;
>>>> +       } else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
>>>> +               int ret_btf_id;
>>>> +
>>>> +               mark_reg_known_zero(env, regs, BPF_REG_0);
>>>> +               regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
>>>> +               ret_btf_id = *fn->ret_btf_id;
>>>
>>> might be a good idea to check fb->ret_btf_id for NULL and print a
>>> warning + return -EFAULT. It's not supposed to happen on properly
>>> configured kernel, but during development this will save a bunch of
>>> time and frustration for next person trying to add something with
>>> RET_PTR_TO_BTF_ID_OR_NULL.
>>
>> I would like prefer to delay this with current code. Otherwise,
>> it gives an impression fn->ret_btf_id might be NULL and it is
>> actually never NULL. We can add NULL check if the future change
>> requires it. I am not sure what the future change could be,
>> but you need some way to get the return btf_id, the above is
>> one of them.
> 
> It's not **supposed** to be NULL, same as a bunch of other invariants
> about BPF helper proto definitions, but verifier does check sanity for
> such cases, instead of crashing. But up to you. I'm pretty sure
> someone will trip up on this.

I think there are certain expectation for argument reg_state vs. certain
fields in the structure.

int btf_resolve_helper_id(struct bpf_verifier_log *log,
                           const struct bpf_func_proto *fn, int arg)
{
         int *btf_id = &fn->btf_id[arg];
         int ret;

         if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID)
                 return -EINVAL;

         ret = READ_ONCE(*btf_id);
	...
}

If ARG_PTR_TO_BTF_ID, the verifier did not really check
whether btf_id pointer is valid or not. It just use it.

The same applies to the new return type. If in func_proto,
somebody sets RET_PTR_TO_BTF_ID_OR_NULL, it is expected
that func_proto->ret_btf_id is valid.

Code review or feature selftest should catch errors
if they are out-of-sync.

> 
>>
>>>
>>>> +               if (ret_btf_id == 0) {
>>>
>>> This also has to be struct/union (after typedef/mods stripping, of
>>> course). Or are there other cases?
>>
>> This is an "int". The func_proto difinition is below:
>> int *ret_btf_id; /* return value btf_id */
> 
> I meant the BTF type itself that this btf_id points to. Is there any
> use case where this won't be a pointer to struct/union and instead
> something like a pointer to an int?

Maybe you misunderstood. The mechanism is similar to the argument btf_id 
encoding in func_proto's:

static int bpf_seq_printf_btf_ids[5];
...
         .btf_id         = bpf_seq_printf_btf_ids,

func_proto->ret_btf_id will be a pointer to int which encodes the 
btf_id, not the btf_type.

> 
>>
>>>
>>>> +                       verbose(env, "invalid return type %d of func %s#%d\n",
>>>> +                               fn->ret_type, func_id_name(func_id), func_id);
>>>> +                       return -EINVAL;
>>>> +               }
>>>> +               regs[BPF_REG_0].btf_id = ret_btf_id;
>>>>           } else {
>>>>                   verbose(env, "unknown return type %d of func %s#%d\n",
>>>>                           fn->ret_type, func_id_name(func_id), func_id);
>>>
>>> [...]
>>>
>>>> +void init_btf_sock_ids(struct btf *btf)
>>>> +{
>>>> +       int i, btf_id;
>>>> +
>>>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
>>>> +               btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
>>>> +                                              BTF_KIND_STRUCT);
>>>> +               if (btf_id > 0)
>>>> +                       btf_sock_ids[i] = btf_id;
>>>> +       }
>>>> +}
>>>
>>> This will hopefully go away with Jiri's work on static BTF IDs, right?
>>> So looking forward to that :)
>>
>> Yes. That's the plan.
>>
>>>
>>>> +
>>>> +static bool check_arg_btf_id(u32 btf_id, u32 arg)
>>>> +{
>>>> +       int i;
>>>> +
>>>> +       /* only one argument, no need to check arg */
>>>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
>>>> +               if (btf_sock_ids[i] == btf_id)
>>>> +                       return true;
>>>> +       return false;
>>>> +}
>>>> +
>>>
>>> [...]
>>>
Andrii Nakryiko June 23, 2020, 8:11 p.m. UTC | #7 
On Tue, Jun 23, 2020 at 12:47 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 6/23/20 11:23 AM, Andrii Nakryiko wrote:
> > On Tue, Jun 23, 2020 at 7:52 AM Yonghong Song <yhs@fb.com> wrote:
> >>
> >>
> >>
> >> On 6/22/20 11:39 PM, Andrii Nakryiko wrote:
> >>> On Mon, Jun 22, 2020 at 5:38 PM Yonghong Song <yhs@fb.com> wrote:
> >>>>
> >>>> The helper is used in tracing programs to cast a socket
> >>>> pointer to a tcp6_sock pointer.
> >>>> The return value could be NULL if the casting is illegal.
> >>>>
> >>>> A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
> >>>> so the verifier is able to deduce proper return types for the helper.
> >>>>
> >>>> Different from the previous BTF_ID based helpers,
> >>>> the bpf_skc_to_tcp6_sock() argument can be several possible
> >>>> btf_ids. More specifically, all possible socket data structures
> >>>> with sock_common appearing in the first in the memory layout.
> >>>> This patch only added socket types related to tcp and udp.
> >>>>
> >>>> All possible argument btf_id and return value btf_id
> >>>> for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
> >>>> cached. In the future, it is even possible to precompute
> >>>> these btf_id's at kernel build time.
> >>>>
> >>>> Acked-by: Martin KaFai Lau <kafai@fb.com>
> >>>> Signed-off-by: Yonghong Song <yhs@fb.com>
> >>>> ---
> >>>
> >>> Looks good to me as is, but see a few suggestions, they will probably
> >>> save me time at some point as well :)
> >>>
> >>> Acked-by: Andrii Nakryiko <andriin@fb.com>
> >>>
> >>>
> >>>>    include/linux/bpf.h            | 12 +++++
> >>>>    include/uapi/linux/bpf.h       |  9 +++-
> >>>>    kernel/bpf/btf.c               |  1 +
> >>>>    kernel/bpf/verifier.c          | 43 +++++++++++++-----
> >>>>    kernel/trace/bpf_trace.c       |  2 +
> >>>>    net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
> >>>>    scripts/bpf_helpers_doc.py     |  2 +
> >>>>    tools/include/uapi/linux/bpf.h |  9 +++-
> >>>>    8 files changed, 146 insertions(+), 12 deletions(-)
> >>>>
> >>>
> >>> [...]
> >>>
> >>>> @@ -4815,6 +4826,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
> >>>>                   regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
> >>>>                   regs[BPF_REG_0].id = ++env->id_gen;
> >>>>                   regs[BPF_REG_0].mem_size = meta.mem_size;
> >>>> +       } else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
> >>>> +               int ret_btf_id;
> >>>> +
> >>>> +               mark_reg_known_zero(env, regs, BPF_REG_0);
> >>>> +               regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
> >>>> +               ret_btf_id = *fn->ret_btf_id;
> >>>
> >>> might be a good idea to check fb->ret_btf_id for NULL and print a
> >>> warning + return -EFAULT. It's not supposed to happen on properly
> >>> configured kernel, but during development this will save a bunch of
> >>> time and frustration for next person trying to add something with
> >>> RET_PTR_TO_BTF_ID_OR_NULL.
> >>
> >> I would like prefer to delay this with current code. Otherwise,
> >> it gives an impression fn->ret_btf_id might be NULL and it is
> >> actually never NULL. We can add NULL check if the future change
> >> requires it. I am not sure what the future change could be,
> >> but you need some way to get the return btf_id, the above is
> >> one of them.
> >
> > It's not **supposed** to be NULL, same as a bunch of other invariants
> > about BPF helper proto definitions, but verifier does check sanity for
> > such cases, instead of crashing. But up to you. I'm pretty sure
> > someone will trip up on this.
>
> I think there are certain expectation for argument reg_state vs. certain
> fields in the structure.
>
> int btf_resolve_helper_id(struct bpf_verifier_log *log,
>                            const struct bpf_func_proto *fn, int arg)
> {
>          int *btf_id = &fn->btf_id[arg];
>          int ret;
>
>          if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID)
>                  return -EINVAL;
>
>          ret = READ_ONCE(*btf_id);
>         ...
> }
>
> If ARG_PTR_TO_BTF_ID, the verifier did not really check
> whether btf_id pointer is valid or not. It just use it.

Right, it's not a universal rule. But grep for "misconfigured" in
kernel/bpf/verifier.c to see a bunch of places where the verifier
could crash on NULL dereference, but instead emits an error message
and returns failure.

This was a suggestion, I'll stop asking for this :)

>
> The same applies to the new return type. If in func_proto,
> somebody sets RET_PTR_TO_BTF_ID_OR_NULL, it is expected
> that func_proto->ret_btf_id is valid.
>
> Code review or feature selftest should catch errors
> if they are out-of-sync.
>
> >
> >>
> >>>
> >>>> +               if (ret_btf_id == 0) {
> >>>
> >>> This also has to be struct/union (after typedef/mods stripping, of
> >>> course). Or are there other cases?
> >>
> >> This is an "int". The func_proto difinition is below:
> >> int *ret_btf_id; /* return value btf_id */
> >
> > I meant the BTF type itself that this btf_id points to. Is there any
> > use case where this won't be a pointer to struct/union and instead
> > something like a pointer to an int?
>
> Maybe you misunderstood. The mechanism is similar to the argument btf_id
> encoding in func_proto's:
>
> static int bpf_seq_printf_btf_ids[5];
> ...
>          .btf_id         = bpf_seq_printf_btf_ids,
>
> func_proto->ret_btf_id will be a pointer to int which encodes the
> btf_id, not the btf_type.

I understand that. Say it points to value 25. BTF type with ID=25 is
going to be BTF_KIND_PTR -> BTF_KIND_STRUCT. I was wondering if we
want/need to check that it's always BTF_KIND_PTR -> (modifier)* ->
BTF_KIND_STRUCT/BTF_KIND_UNION. That's it.

>
> >
> >>
> >>>
> >>>> +                       verbose(env, "invalid return type %d of func %s#%d\n",
> >>>> +                               fn->ret_type, func_id_name(func_id), func_id);
> >>>> +                       return -EINVAL;
> >>>> +               }
> >>>> +               regs[BPF_REG_0].btf_id = ret_btf_id;
> >>>>           } else {
> >>>>                   verbose(env, "unknown return type %d of func %s#%d\n",
> >>>>                           fn->ret_type, func_id_name(func_id), func_id);
> >>>
> >>> [...]
> >>>
> >>>> +void init_btf_sock_ids(struct btf *btf)
> >>>> +{
> >>>> +       int i, btf_id;
> >>>> +
> >>>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
> >>>> +               btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
> >>>> +                                              BTF_KIND_STRUCT);
> >>>> +               if (btf_id > 0)
> >>>> +                       btf_sock_ids[i] = btf_id;
> >>>> +       }
> >>>> +}
> >>>
> >>> This will hopefully go away with Jiri's work on static BTF IDs, right?
> >>> So looking forward to that :)
> >>
> >> Yes. That's the plan.
> >>
> >>>
> >>>> +
> >>>> +static bool check_arg_btf_id(u32 btf_id, u32 arg)
> >>>> +{
> >>>> +       int i;
> >>>> +
> >>>> +       /* only one argument, no need to check arg */
> >>>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
> >>>> +               if (btf_sock_ids[i] == btf_id)
> >>>> +                       return true;
> >>>> +       return false;
> >>>> +}
> >>>> +
> >>>
> >>> [...]
> >>>
Yonghong Song June 23, 2020, 8:46 p.m. UTC | #8 
On 6/23/20 1:11 PM, Andrii Nakryiko wrote:
> On Tue, Jun 23, 2020 at 12:47 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 6/23/20 11:23 AM, Andrii Nakryiko wrote:
>>> On Tue, Jun 23, 2020 at 7:52 AM Yonghong Song <yhs@fb.com> wrote:
>>>>
>>>>
>>>>
>>>> On 6/22/20 11:39 PM, Andrii Nakryiko wrote:
>>>>> On Mon, Jun 22, 2020 at 5:38 PM Yonghong Song <yhs@fb.com> wrote:
>>>>>>
>>>>>> The helper is used in tracing programs to cast a socket
>>>>>> pointer to a tcp6_sock pointer.
>>>>>> The return value could be NULL if the casting is illegal.
>>>>>>
>>>>>> A new helper return type RET_PTR_TO_BTF_ID_OR_NULL is added
>>>>>> so the verifier is able to deduce proper return types for the helper.
>>>>>>
>>>>>> Different from the previous BTF_ID based helpers,
>>>>>> the bpf_skc_to_tcp6_sock() argument can be several possible
>>>>>> btf_ids. More specifically, all possible socket data structures
>>>>>> with sock_common appearing in the first in the memory layout.
>>>>>> This patch only added socket types related to tcp and udp.
>>>>>>
>>>>>> All possible argument btf_id and return value btf_id
>>>>>> for helper bpf_skc_to_tcp6_sock() are pre-calculcated and
>>>>>> cached. In the future, it is even possible to precompute
>>>>>> these btf_id's at kernel build time.
>>>>>>
>>>>>> Acked-by: Martin KaFai Lau <kafai@fb.com>
>>>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>>>>> ---
>>>>>
>>>>> Looks good to me as is, but see a few suggestions, they will probably
>>>>> save me time at some point as well :)
>>>>>
>>>>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>>>>>
>>>>>
>>>>>>     include/linux/bpf.h            | 12 +++++
>>>>>>     include/uapi/linux/bpf.h       |  9 +++-
>>>>>>     kernel/bpf/btf.c               |  1 +
>>>>>>     kernel/bpf/verifier.c          | 43 +++++++++++++-----
>>>>>>     kernel/trace/bpf_trace.c       |  2 +
>>>>>>     net/core/filter.c              | 80 ++++++++++++++++++++++++++++++++++
>>>>>>     scripts/bpf_helpers_doc.py     |  2 +
>>>>>>     tools/include/uapi/linux/bpf.h |  9 +++-
>>>>>>     8 files changed, 146 insertions(+), 12 deletions(-)
>>>>>>
>>>>>
>>>>> [...]
>>>>>
>>>>>> @@ -4815,6 +4826,18 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>>>>>>                    regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
>>>>>>                    regs[BPF_REG_0].id = ++env->id_gen;
>>>>>>                    regs[BPF_REG_0].mem_size = meta.mem_size;
>>>>>> +       } else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
>>>>>> +               int ret_btf_id;
>>>>>> +
>>>>>> +               mark_reg_known_zero(env, regs, BPF_REG_0);
>>>>>> +               regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
>>>>>> +               ret_btf_id = *fn->ret_btf_id;
[...]
>>>
>>>>
>>>>>
>>>>>> +               if (ret_btf_id == 0) {
>>>>>
>>>>> This also has to be struct/union (after typedef/mods stripping, of
>>>>> course). Or are there other cases?
>>>>
>>>> This is an "int". The func_proto difinition is below:
>>>> int *ret_btf_id; /* return value btf_id */
>>>
>>> I meant the BTF type itself that this btf_id points to. Is there any
>>> use case where this won't be a pointer to struct/union and instead
>>> something like a pointer to an int?
>>
>> Maybe you misunderstood. The mechanism is similar to the argument btf_id
>> encoding in func_proto's:
>>
>> static int bpf_seq_printf_btf_ids[5];
>> ...
>>           .btf_id         = bpf_seq_printf_btf_ids,
>>
>> func_proto->ret_btf_id will be a pointer to int which encodes the
>> btf_id, not the btf_type.
> 
> I understand that. Say it points to value 25. BTF type with ID=25 is
> going to be BTF_KIND_PTR -> BTF_KIND_STRUCT. I was wondering if we
> want/need to check that it's always BTF_KIND_PTR -> (modifier)* ->
> BTF_KIND_STRUCT/BTF_KIND_UNION. That's it.

Just to be clear. The ret_btf_id returned here is the btf id is the
type id of the pointee, so in this case it is BTF_KIND_STRUCT/....

These id's are pre-calculated and stored in memory. Unless the whole
thing is mess up, there is no need to check...

> 
>>
>>>
>>>>
>>>>>
>>>>>> +                       verbose(env, "invalid return type %d of func %s#%d\n",
>>>>>> +                               fn->ret_type, func_id_name(func_id), func_id);
>>>>>> +                       return -EINVAL;
>>>>>> +               }
>>>>>> +               regs[BPF_REG_0].btf_id = ret_btf_id;
>>>>>>            } else {
>>>>>>                    verbose(env, "unknown return type %d of func %s#%d\n",
>>>>>>                            fn->ret_type, func_id_name(func_id), func_id);
>>>>>
>>>>> [...]
>>>>>
>>>>>> +void init_btf_sock_ids(struct btf *btf)
>>>>>> +{
>>>>>> +       int i, btf_id;
>>>>>> +
>>>>>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
>>>>>> +               btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
>>>>>> +                                              BTF_KIND_STRUCT);
>>>>>> +               if (btf_id > 0)
>>>>>> +                       btf_sock_ids[i] = btf_id;
>>>>>> +       }
>>>>>> +}
>>>>>
>>>>> This will hopefully go away with Jiri's work on static BTF IDs, right?
>>>>> So looking forward to that :)
>>>>
>>>> Yes. That's the plan.
>>>>
>>>>>
>>>>>> +
>>>>>> +static bool check_arg_btf_id(u32 btf_id, u32 arg)
>>>>>> +{
>>>>>> +       int i;
>>>>>> +
>>>>>> +       /* only one argument, no need to check arg */
>>>>>> +       for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
>>>>>> +               if (btf_sock_ids[i] == btf_id)
>>>>>> +                       return true;
>>>>>> +       return false;
>>>>>> +}
>>>>>> +
>>>>>
>>>>> [...]
>>>>>
diff mbox series

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1e1501ee53ce..e2b9f2075e5b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -265,6 +265,7 @@  enum bpf_return_type {
 	RET_PTR_TO_TCP_SOCK_OR_NULL,	/* returns a pointer to a tcp_sock or NULL */
 	RET_PTR_TO_SOCK_COMMON_OR_NULL,	/* returns a pointer to a sock_common or NULL */
 	RET_PTR_TO_ALLOC_MEM_OR_NULL,	/* returns a pointer to dynamically allocated memory or NULL */
+	RET_PTR_TO_BTF_ID_OR_NULL,	/* returns a pointer to a btf_id or NULL */
 };
 
 /* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
@@ -287,6 +288,12 @@  struct bpf_func_proto {
 		enum bpf_arg_type arg_type[5];
 	};
 	int *btf_id; /* BTF ids of arguments */
+	bool (*check_btf_id)(u32 btf_id, u32 arg); /* if the argument btf_id is
+						    * valid. Often used if more
+						    * than one btf id is permitted
+						    * for this argument.
+						    */
+	int *ret_btf_id; /* return value btf_id */
 };
 
 /* bpf_context is intentionally undefined structure. Pointer to bpf_context is
@@ -1638,6 +1645,7 @@  extern const struct bpf_func_proto bpf_ringbuf_reserve_proto;
 extern const struct bpf_func_proto bpf_ringbuf_submit_proto;
 extern const struct bpf_func_proto bpf_ringbuf_discard_proto;
 extern const struct bpf_func_proto bpf_ringbuf_query_proto;
+extern const struct bpf_func_proto bpf_skc_to_tcp6_sock_proto;
 
 const struct bpf_func_proto *bpf_tracing_func_proto(
 	enum bpf_func_id func_id, const struct bpf_prog *prog);
@@ -1661,6 +1669,7 @@  u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
 				struct bpf_insn *insn_buf,
 				struct bpf_prog *prog,
 				u32 *target_size);
+void init_btf_sock_ids(struct btf *btf);
 #else
 static inline bool bpf_sock_common_is_valid_access(int off, int size,
 						   enum bpf_access_type type,
@@ -1682,6 +1691,9 @@  static inline u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
 {
 	return 0;
 }
+static inline void init_btf_sock_ids(struct btf *btf)
+{
+}
 #endif
 
 #ifdef CONFIG_INET
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 19684813faae..394fcba27b6a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3252,6 +3252,12 @@  union bpf_attr {
  * 		case of **BPF_CSUM_LEVEL_QUERY**, the current skb->csum_level
  * 		is returned or the error code -EACCES in case the skb is not
  * 		subject to CHECKSUM_UNNECESSARY.
+ *
+ * struct tcp6_sock *bpf_skc_to_tcp6_sock(void *sk)
+ *	Description
+ *		Dynamically cast a *sk* pointer to a *tcp6_sock* pointer.
+ *	Return
+ *		*sk* if casting is valid, or NULL otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3389,7 +3395,8 @@  union bpf_attr {
 	FN(ringbuf_submit),		\
 	FN(ringbuf_discard),		\
 	FN(ringbuf_query),		\
-	FN(csum_level),
+	FN(csum_level),			\
+	FN(skc_to_tcp6_sock),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index e377d1981730..4c3007f428b1 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -3674,6 +3674,7 @@  struct btf *btf_parse_vmlinux(void)
 		goto errout;
 
 	bpf_struct_ops_init(btf, log);
+	init_btf_sock_ids(btf);
 
 	btf_verifier_env_free(env);
 	refcount_set(&btf->refcnt, 1);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7460f967cb75..7de98906ddf4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3800,12 +3800,14 @@  static int int_ptr_type_to_size(enum bpf_arg_type type)
 	return -EINVAL;
 }
 
-static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
-			  enum bpf_arg_type arg_type,
-			  struct bpf_call_arg_meta *meta)
+static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
+			  struct bpf_call_arg_meta *meta,
+			  const struct bpf_func_proto *fn)
 {
+	u32 regno = BPF_REG_1 + arg;
 	struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
 	enum bpf_reg_type expected_type, type = reg->type;
+	enum bpf_arg_type arg_type = fn->arg_type[arg];
 	int err = 0;
 
 	if (arg_type == ARG_DONTCARE)
@@ -3885,9 +3887,16 @@  static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 		expected_type = PTR_TO_BTF_ID;
 		if (type != expected_type)
 			goto err_type;
-		if (reg->btf_id != meta->btf_id) {
-			verbose(env, "Helper has type %s got %s in R%d\n",
-				kernel_type_name(meta->btf_id),
+		if (!fn->check_btf_id) {
+			if (reg->btf_id != meta->btf_id) {
+				verbose(env, "Helper has type %s got %s in R%d\n",
+					kernel_type_name(meta->btf_id),
+					kernel_type_name(reg->btf_id), regno);
+
+				return -EACCES;
+			}
+		} else if (!fn->check_btf_id(reg->btf_id, arg)) {
+			verbose(env, "Helper does not support %s in R%d\n",
 				kernel_type_name(reg->btf_id), regno);
 
 			return -EACCES;
@@ -4709,10 +4718,12 @@  static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 	meta.func_id = func_id;
 	/* check args */
 	for (i = 0; i < 5; i++) {
-		err = btf_resolve_helper_id(&env->log, fn, i);
-		if (err > 0)
-			meta.btf_id = err;
-		err = check_func_arg(env, BPF_REG_1 + i, fn->arg_type[i], &meta);
+		if (!fn->check_btf_id) {
+			err = btf_resolve_helper_id(&env->log, fn, i);
+			if (err > 0)
+				meta.btf_id = err;
+		}
+		err = check_func_arg(env, i, &meta, fn);
 		if (err)
 			return err;
 	}
@@ -4815,6 +4826,18 @@  static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
 		regs[BPF_REG_0].type = PTR_TO_MEM_OR_NULL;
 		regs[BPF_REG_0].id = ++env->id_gen;
 		regs[BPF_REG_0].mem_size = meta.mem_size;
+	} else if (fn->ret_type == RET_PTR_TO_BTF_ID_OR_NULL) {
+		int ret_btf_id;
+
+		mark_reg_known_zero(env, regs, BPF_REG_0);
+		regs[BPF_REG_0].type = PTR_TO_BTF_ID_OR_NULL;
+		ret_btf_id = *fn->ret_btf_id;
+		if (ret_btf_id == 0) {
+			verbose(env, "invalid return type %d of func %s#%d\n",
+				fn->ret_type, func_id_name(func_id), func_id);
+			return -EINVAL;
+		}
+		regs[BPF_REG_0].btf_id = ret_btf_id;
 	} else {
 		verbose(env, "unknown return type %d of func %s#%d\n",
 			fn->ret_type, func_id_name(func_id), func_id);
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index afaec7e082d9..478c10d1ec33 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1515,6 +1515,8 @@  tracing_prog_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_skb_output_proto;
 	case BPF_FUNC_xdp_output:
 		return &bpf_xdp_output_proto;
+	case BPF_FUNC_skc_to_tcp6_sock:
+		return &bpf_skc_to_tcp6_sock_proto;
 #endif
 	case BPF_FUNC_seq_printf:
 		return prog->expected_attach_type == BPF_TRACE_ITER ?
diff --git a/net/core/filter.c b/net/core/filter.c
index 73395384afe2..32efc1fc16cf 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -47,6 +47,7 @@ 
 #include <linux/seccomp.h>
 #include <linux/if_vlan.h>
 #include <linux/bpf.h>
+#include <linux/btf.h>
 #include <net/sch_generic.h>
 #include <net/cls_cgroup.h>
 #include <net/dst_metadata.h>
@@ -9191,3 +9192,82 @@  void bpf_prog_change_xdp(struct bpf_prog *prev_prog, struct bpf_prog *prog)
 {
 	bpf_dispatcher_change_prog(BPF_DISPATCHER_PTR(xdp), prev_prog, prog);
 }
+
+/* Define a list of socket types which can be the argument for
+ * skc_to_*_sock() helpers. All these sockets should have
+ * sock_common as the first argument in its memory layout.
+ */
+#define BTF_SOCK_TYPE_xxx \
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_INET, "inet_sock")			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_INET_CONN, "inet_connection_sock")	\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_INET_REQ, "inet_request_sock")	\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_INET_TW, "inet_timewait_sock")	\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_REQ, "request_sock")		\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_SOCK, "sock")			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_SOCK_COMMON, "sock_common")		\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP, "tcp_sock")			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP_REQ, "tcp_request_sock")	\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP_TW, "tcp_timewait_sock")	\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_TCP6, "tcp6_sock")			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP, "udp_sock")			\
+	BTF_SOCK_TYPE(BTF_SOCK_TYPE_UDP6, "udp6_sock")
+
+enum {
+#define BTF_SOCK_TYPE(name, str) name,
+BTF_SOCK_TYPE_xxx
+#undef BTF_SOCK_TYPE
+MAX_BTF_SOCK_TYPE,
+};
+
+static const char *bpf_sock_types[] = {
+#define BTF_SOCK_TYPE(name, str) str,
+BTF_SOCK_TYPE_xxx
+#undef BTF_SOCK_TYPE
+};
+
+static int btf_sock_ids[MAX_BTF_SOCK_TYPE];
+
+void init_btf_sock_ids(struct btf *btf)
+{
+	int i, btf_id;
+
+	for (i = 0; i < MAX_BTF_SOCK_TYPE; i++) {
+		btf_id = btf_find_by_name_kind(btf, bpf_sock_types[i],
+					       BTF_KIND_STRUCT);
+		if (btf_id > 0)
+			btf_sock_ids[i] = btf_id;
+	}
+}
+
+static bool check_arg_btf_id(u32 btf_id, u32 arg)
+{
+	int i;
+
+	/* only one argument, no need to check arg */
+	for (i = 0; i < MAX_BTF_SOCK_TYPE; i++)
+		if (btf_sock_ids[i] == btf_id)
+			return true;
+	return false;
+}
+
+BPF_CALL_1(bpf_skc_to_tcp6_sock, struct sock *, sk)
+{
+	/* tcp6_sock type is not generated in dwarf and hence btf,
+	 * trigger an explicit type generation here.
+	 */
+	BTF_TYPE_EMIT(struct tcp6_sock);
+	if (sk_fullsock(sk) && sk->sk_protocol == IPPROTO_TCP &&
+	    sk->sk_family == AF_INET6)
+		return (unsigned long)sk;
+
+	return (unsigned long)NULL;
+}
+
+const struct bpf_func_proto bpf_skc_to_tcp6_sock_proto = {
+	.func			= bpf_skc_to_tcp6_sock,
+	.gpl_only		= false,
+	.ret_type		= RET_PTR_TO_BTF_ID_OR_NULL,
+	.arg1_type		= ARG_PTR_TO_BTF_ID,
+	.check_btf_id		= check_arg_btf_id,
+	.ret_btf_id		= &btf_sock_ids[BTF_SOCK_TYPE_TCP6],
+};
diff --git a/scripts/bpf_helpers_doc.py b/scripts/bpf_helpers_doc.py
index 91fa668fa860..6c2f64118651 100755
--- a/scripts/bpf_helpers_doc.py
+++ b/scripts/bpf_helpers_doc.py
@@ -421,6 +421,7 @@  class PrinterHelpers(Printer):
             'struct sockaddr',
             'struct tcphdr',
             'struct seq_file',
+            'struct tcp6_sock',
 
             'struct __sk_buff',
             'struct sk_msg_md',
@@ -458,6 +459,7 @@  class PrinterHelpers(Printer):
             'struct sockaddr',
             'struct tcphdr',
             'struct seq_file',
+            'struct tcp6_sock',
     }
     mapped_types = {
             'u8': '__u8',
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 19684813faae..394fcba27b6a 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3252,6 +3252,12 @@  union bpf_attr {
  * 		case of **BPF_CSUM_LEVEL_QUERY**, the current skb->csum_level
  * 		is returned or the error code -EACCES in case the skb is not
  * 		subject to CHECKSUM_UNNECESSARY.
+ *
+ * struct tcp6_sock *bpf_skc_to_tcp6_sock(void *sk)
+ *	Description
+ *		Dynamically cast a *sk* pointer to a *tcp6_sock* pointer.
+ *	Return
+ *		*sk* if casting is valid, or NULL otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -3389,7 +3395,8 @@  union bpf_attr {
 	FN(ringbuf_submit),		\
 	FN(ringbuf_discard),		\
 	FN(ringbuf_query),		\
-	FN(csum_level),
+	FN(csum_level),			\
+	FN(skc_to_tcp6_sock),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call