[bug report] bpf: Implement dynptr copy kfuncs

[bug report] bpf: Implement dynptr copy kfuncs

[Dan Carpenter]

Hello Mykyta Yatsenko,

Commit a498ee7576de ("bpf: Implement dynptr copy kfuncs") from May
12, 2025 (linux-next), leads to the following Smatch static checker
warning:

	kernel/trace/bpf_trace.c:3557 copy_user_data_sleepable()
	warn: maybe return -EFAULT instead of the bytes remaining?

kernel/trace/bpf_trace.c
    3551 static __always_inline int copy_user_data_sleepable(void *dst, const void *unsafe_src,
    3552                                                     u32 size, struct task_struct *tsk)
    3553 {
    3554         int ret;
    3555 
    3556         if (!tsk) /* Read from the current task */
--> 3557                 return copy_from_user(dst, (const void __user *)unsafe_src, size);

I don't know if it matters what we return here so long as it's non-zero.
This is probably a fast path so maybe we're returning positives
intentionally?

    3558 
    3559         ret = access_process_vm(tsk, (unsigned long)unsafe_src, dst, size, 0);
    3560         if (ret != size)
    3561                 return -EFAULT;
    3562         return 0;
    3563 }

regards,
dan carpenter

Re: [bug report] bpf: Implement dynptr copy kfuncs

[Mykyta Yatsenko]

On 5/23/25 16:58, Dan Carpenter wrote:
> Hello Mykyta Yatsenko,
>
> Commit a498ee7576de ("bpf: Implement dynptr copy kfuncs") from May
> 12, 2025 (linux-next), leads to the following Smatch static checker
> warning:
>
> 	kernel/trace/bpf_trace.c:3557 copy_user_data_sleepable()
> 	warn: maybe return -EFAULT instead of the bytes remaining?
>
> kernel/trace/bpf_trace.c
>      3551 static __always_inline int copy_user_data_sleepable(void *dst, const void *unsafe_src,
>      3552                                                     u32 size, struct task_struct *tsk)
>      3553 {
>      3554         int ret;
>      3555
>      3556         if (!tsk) /* Read from the current task */
> --> 3557                 return copy_from_user(dst, (const void __user *)unsafe_src, size);
>
> I don't know if it matters what we return here so long as it's non-zero.
> This is probably a fast path so maybe we're returning positives
> intentionally?
>
>      3558
>      3559         ret = access_process_vm(tsk, (unsigned long)unsafe_src, dst, size, 0);
>      3560         if (ret != size)
>      3561                 return -EFAULT;
>      3562         return 0;
>      3563 }
>
> regards,
> dan carpenter
>
Hi, thanks for the report, I think the warning is right, let me send the 
fix a bit later today.