>> ...
>>> kernel/bpf/verifier.c | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>>
>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>> index 3303a3605ee8..6481604ab612 100644
>>> --- a/kernel/bpf/verifier.c
>>> +++ b/kernel/bpf/verifier.c
>>> @@ -7788,6 +7788,12 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
>>> static int check_atomic_load(struct bpf_verifier_env *env,
>>> struct bpf_insn *insn)
>>> {
>>> + int err;
>>> +
>>> + err = check_reg_arg(env, insn->src_reg, SRC_OP);
>>> + if (err)
>>> + return err;
>>> +
>>
>>I agree with these changes, however, both check_load_mem() and
>>check_store_reg() already do check_reg_arg() first thing.
>>Maybe just swap the atomic_ptr_type_ok() and check_load_mem()?
>
>You're absolutely right. The additional check_reg_arg() introduces some
>redundancy since check_load_mem() and check_store_reg() do that.
IMHO, in this specific case, I believe OOB is caused by invalid register
number in reg_state(), so check_reg_arg() is too much and instead just
checking register number before atomic_ptr_type_ok() is sufficient.
Although it's still somewhat redundant and not a fundamental solution, I
think it would be better than doing whole register checking by
check_reg_arg().
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 3303a3605ee8..48131839ac26 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7788,6 +7788,11 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
static int check_atomic_load(struct bpf_verifier_env *env,
struct bpf_insn *insn)
{
+ if (insn->src_reg >= MAX_BPF_REG) {
+ verbose(env, "R%d is invalid\n", insn->src_reg);
+ return -EINVAL;
+ }
+
if (!atomic_ptr_type_ok(env, insn->src_reg, insn)) {
verbose(env, "BPF_ATOMIC loads from R%d %s is not allowed\n",
insn->src_reg,
@@ -7801,6 +7806,11 @@ static int check_atomic_load(struct bpf_verifier_env *env,
static int check_atomic_store(struct bpf_verifier_env *env,
struct bpf_insn *insn)
{
+ if (insn->dst_reg >= MAX_BPF_REG) {
+ verbose(env, "R%d is invalid\n", insn->dst_reg);
+ return -EINVAL;
+ }
+
if (!atomic_ptr_type_ok(env, insn->dst_reg, insn)) {
verbose(env, "BPF_ATOMIC stores into R%d %s is not allowed\n",
insn->dst_reg,
>I've revised the patch to simply swap the order and syzbot didn't trigger
>the issue in this context.
> https://lore.kernel.org/all/20250315055941.10487-2-enjuk@xxxxxxxxxx/
> ...
Regards,
Kohei
