On Wed, Jul 19, 2023 at 03:58:59PM +0800, Menglong Dong wrote:
> > 1957 /* copy function arguments from origin stack frame
> > 1958 * into current stack frame.
> > 1959 *
> > 1960 * The starting address of the arguments on-stack
> > 1961 * is:
> > 1962 * rbp + 8(push rbp) +
> > 1963 * 8(return addr of origin call) +
> > 1964 * 8(return addr of the caller)
> > 1965 * which means: rbp + 24
> > 1966 */
> > 1967 for (j = 0; j < arg_regs; j++) {
> > 1968 emit_ldx(prog, BPF_DW, BPF_REG_0, BPF_REG_FP,
> > 1969 nr_stack_slots * 8 + 0x18);
> > 1970 emit_stx(prog, BPF_DW, BPF_REG_FP, BPF_REG_0,
> > 1971 -stack_size);
> > 1972
> > 1973 if (!nr_stack_slots)
> > 1974 first_off = stack_size;
> > 1975 stack_size -= 8;
> > 1976 nr_stack_slots++;
> > 1977 }
> > 1978 } else {
> > 1979 /* Only copy the arguments on-stack to current
> > 1980 * 'stack_size' and ignore the regs, used to
> > 1981 * prepare the arguments on-stack for orign call.
> > 1982 */
> > 1983 if (for_call_origin) {
> > 1984 nr_regs += arg_regs;
> > 1985 continue;
> > 1986 }
> > 1987
> > 1988 /* copy the arguments from regs into stack */
> > 1989 for (j = 0; j < arg_regs; j++) {
> > 1990 emit_stx(prog, BPF_DW, BPF_REG_FP,
> > 1991 nr_regs == 5 ? X86_REG_R9 : BPF_REG_1 + nr_regs,
> > 1992 -stack_size);
> > 1993 stack_size -= 8;
> > 1994 nr_regs++;
> > 1995 }
> > 1996 }
> > 1997 }
> > 1998
> > --> 1999 clean_stack_garbage(m, prog, nr_stack_slots, first_off);
> > 2000 }
>
> Hello,
>
> Thanks for the reporting. The variable ‘first_off’ that passed to
> clean_stack_garbage() should be ok, as it is only used when
> "nr_stack_slots == 1”, in which case “first_off” should already be
> initialized.
>
> (Anyway, maybe we should initialize it to avoid passing a
> uninitialized variable to a function?)
There is no downside to initializing it to zero. Everyone does that
automatically these days.
In a case where the variable is not used, it depends on if the function
is inlined or not. If it's inlined, then it's not a bug. If it isn't
inlined then it's still a bug whether the variable is used or not.
It's undefined behavior in C and it can cause runtime warnings with
KMEMsan.
regards,
dan carpenter
