Re: [PATCH v2 bpf-next 01/13] bpf: Support multiple arg regs w/ ref_obj_id for kfuncs

Alexei Starovoitov
2022-12-29T03:24:49Z

On Sat, Dec 17, 2022 at 12:24:54AM -0800, Dave Marchevsky wrote:
> Currently, kfuncs marked KF_RELEASE indicate that they release some
> previously-acquired arg. The verifier assumes that such a function will
> only have one arg reg w/ ref_obj_id set, and that that arg is the one to
> be released. Multiple kfunc arg regs have ref_obj_id set is considered
> an invalid state.
>
> For helpers, RELEASE is used to tag a particular arg in the function
> proto, not the function itself. The arg with OBJ_RELEASE type tag is the
> arg that the helper will release. There can only be one such tagged arg.
> When verifying arg regs, multiple helper arg regs w/ ref_obj_id set is
> also considered an invalid state.
>
> Later patches in this series will result in some linked_list helpers
> marked KF_RELEASE having a valid reason to take two ref_obj_id args.
> Specifically, bpf_list_push_{front,back} can push a node to a list head
> which is itself part of a list node. In such a scenario both arguments
> to these functions would have ref_obj_id > 0, thus would fail
> verification under current logic.

Well, I think this patch is unnecessary, because there is really no need
to mark lish_push as KF_RELEASE.
The verifier still has to do custom checks for both arguments:
list_node and list_head.
They are different enough. The 'generalization' via
KF_RELEASE | KF_RELEASE_NON_OWN is quite confusing.
Especially considering how register is being picked: 1st vs 2nd.
More details on this in the other reply to patch 2.