[PATCH v2] docs/bpf: clarify ret handling in LSM BPF programs

Ariel Silver posted 1 patch 3 weeks ago
Documentation/bpf/prog_lsm.rst | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
[PATCH v2] docs/bpf: clarify ret handling in LSM BPF programs
Posted by Ariel Silver 3 weeks ago
v2: Fixed trailing whitespace (reported by checkpatch.pl)

Docs currently suggest that all attached BPF LSM programs always run
and that ret simply carries the previous return code. In reality,
execution stops as soon as one program returns non-zero. This is
because call_int_hook() breaks out of the loop when RC != 0, so later
programs are not executed.

Signed-off-by: arielsilver77@gmail.com <arielsilver77@gmail.com>
---
 Documentation/bpf/prog_lsm.rst | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/Documentation/bpf/prog_lsm.rst b/Documentation/bpf/prog_lsm.rst
index ad2be02f3..92bfb64c2 100644
--- a/Documentation/bpf/prog_lsm.rst
+++ b/Documentation/bpf/prog_lsm.rst
@@ -66,21 +66,17 @@ example:

    SEC("lsm/file_mprotect")
    int BPF_PROG(mprotect_audit, struct vm_area_struct *vma,
-            unsigned long reqprot, unsigned long prot, int ret)
+            unsigned long reqprot, unsigned long prot)
    {
-       /* ret is the return value from the previous BPF program
-        * or 0 if it's the first hook.
-        */
-       if (ret != 0)
-           return ret;
-
        int is_heap;

        is_heap = (vma->vm_start >= vma->vm_mm->start_brk &&
               vma->vm_end <= vma->vm_mm->brk);

        /* Return an -EPERM or write information to the perf events buffer
-        * for auditing
+        * for auditing.
+        * Returning a non-zero value will stop the chain of
+        * LSM BPF programs attached to the same hook.
         */
        if (is_heap)
            return -EPERM;
-- 
2.50.1
Re: [PATCH v2] docs/bpf: clarify ret handling in LSM BPF programs
Posted by KP Singh 2 weeks, 1 day ago
On Thu, Sep 11, 2025 at 12:52 PM Ariel Silver <arielsilver77@gmail.com> wrote:
>
> v2: Fixed trailing whitespace (reported by checkpatch.pl)
>
> Docs currently suggest that all attached BPF LSM programs always run
> and that ret simply carries the previous return code. In reality,
> execution stops as soon as one program returns non-zero. This is
> because call_int_hook() breaks out of the loop when RC != 0, so later
> programs are not executed.
>
> Signed-off-by: arielsilver77@gmail.com <arielsilver77@gmail.com>
> ---
>  Documentation/bpf/prog_lsm.rst | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/bpf/prog_lsm.rst b/Documentation/bpf/prog_lsm.rst
> index ad2be02f3..92bfb64c2 100644
> --- a/Documentation/bpf/prog_lsm.rst
> +++ b/Documentation/bpf/prog_lsm.rst
> @@ -66,21 +66,17 @@ example:
>
>     SEC("lsm/file_mprotect")
>     int BPF_PROG(mprotect_audit, struct vm_area_struct *vma,
> -            unsigned long reqprot, unsigned long prot, int ret)
> +            unsigned long reqprot, unsigned long prot)
>     {
> -       /* ret is the return value from the previous BPF program
> -        * or 0 if it's the first hook.
> -        */
> -       if (ret != 0)
> -           return ret;
> -

This is correct as of today, the return value is checked implicitly by
the generated trampoline and the next program in the chain is only
called if this is zero as BPF LSM programs use the modify return
trampoline:

in invoke_bpf_mod_ret:

/* mod_ret prog stored return value into [rbp - 8]. Emit:
* if (*(u64 *)(rbp - 8) != 0)
* goto do_fexit;
*/
/* cmp QWORD PTR [rbp - 0x8], 0x0 */
EMIT4(0x48, 0x83, 0x7d, 0xf8); EMIT1(0x00);

Acked-by: KP Singh <kpsingh@kernel.org>
Re: [PATCH v2] docs/bpf: clarify ret handling in LSM BPF programs
Posted by Ariel Silver 2 weeks ago
Just so I understand.
1) The docs are indeed wrong, correct?
2) Any idea why I get an automatic response of "CONFLICTS" when I send
my patch? Even though im 1000% sure there are no conlicts

On Wed, Sep 17, 2025 at 12:31 PM KP Singh <kpsingh@kernel.org> wrote:
>
> On Thu, Sep 11, 2025 at 12:52 PM Ariel Silver <arielsilver77@gmail.com> wrote:
> >
> > v2: Fixed trailing whitespace (reported by checkpatch.pl)
> >
> > Docs currently suggest that all attached BPF LSM programs always run
> > and that ret simply carries the previous return code. In reality,
> > execution stops as soon as one program returns non-zero. This is
> > because call_int_hook() breaks out of the loop when RC != 0, so later
> > programs are not executed.
> >
> > Signed-off-by: arielsilver77@gmail.com <arielsilver77@gmail.com>
> > ---
> >  Documentation/bpf/prog_lsm.rst | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/bpf/prog_lsm.rst b/Documentation/bpf/prog_lsm.rst
> > index ad2be02f3..92bfb64c2 100644
> > --- a/Documentation/bpf/prog_lsm.rst
> > +++ b/Documentation/bpf/prog_lsm.rst
> > @@ -66,21 +66,17 @@ example:
> >
> >     SEC("lsm/file_mprotect")
> >     int BPF_PROG(mprotect_audit, struct vm_area_struct *vma,
> > -            unsigned long reqprot, unsigned long prot, int ret)
> > +            unsigned long reqprot, unsigned long prot)
> >     {
> > -       /* ret is the return value from the previous BPF program
> > -        * or 0 if it's the first hook.
> > -        */
> > -       if (ret != 0)
> > -           return ret;
> > -
>
> This is correct as of today, the return value is checked implicitly by
> the generated trampoline and the next program in the chain is only
> called if this is zero as BPF LSM programs use the modify return
> trampoline:
>
> in invoke_bpf_mod_ret:
>
> /* mod_ret prog stored return value into [rbp - 8]. Emit:
> * if (*(u64 *)(rbp - 8) != 0)
> * goto do_fexit;
> */
> /* cmp QWORD PTR [rbp - 0x8], 0x0 */
> EMIT4(0x48, 0x83, 0x7d, 0xf8); EMIT1(0x00);
>
> Acked-by: KP Singh <kpsingh@kernel.org>
Re: [PATCH v2] docs/bpf: clarify ret handling in LSM BPF programs
Posted by Alexei Starovoitov 1 week, 1 day ago
On Thu, Sep 18, 2025 at 3:03 AM Ariel Silver <arielsilver77@gmail.com> wrote:
>
> Just so I understand.
> 1) The docs are indeed wrong, correct?

It's arguable, but removing if (ret...) check
from the example is worth doing.

> 2) Any idea why I get an automatic response of "CONFLICTS" when I send
> my patch? Even though im 1000% sure there are no conlicts

The patch doesn't apply to bpf-next tree.
Pls use the right tree as a base for your patch.