Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
From:   Jinmeng Zhou <jjjinmeng.zhou@gmail.com>
Date:   Fri, 21 May 2021 22:32:20 +0800
Message-ID: <CAA-qYXhSrMkGXQ69TyC9pOuGO1dCznxw6PU4LTjxLaDKz09iLA@mail.gmail.com>
Subject: A missing security check bug in trace_format_open().
To:     rostedt@goodmis.org, mingo@redhat.com
Cc:     shenwenbosmile@gmail.com, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Dear maintainers,
hi, our team has found a missing check bug on Linux kernel v5.10.7
using static analysis. There is a missing security check bug in
trace_format_open() before calling function seq_open().

Function profile_open() checks security_locked_down() before the
critical function seq_open().
1. //check security_locked_down() ///////////////////////
2. static int profile_open(struct inode *inode, struct file *file)
3. {
4.   int ret;
5.   ret =3D security_locked_down(LOCKDOWN_TRACEFS);
6.   if (ret)
7.     return ret;
8.   return seq_open(file, &profile_seq_op);
9. }

While trace_format_open() does not call this check,
security_locked_down(), and the parameters of calling seq_open() are
similar.
1. static int trace_format_open(struct inode *inode, struct file *file)
2. {
3.   struct seq_file *m;
4.   int ret;
5.
6.   /* Do we want to hide event format files on tracefs lockdown? */
7.
8.   ret =3D seq_open(file, &trace_format_seq_ops);
9.   if (ret < 0)
10.     return ret;
11.
12.   m =3D file->private_data;
13.   m->private =3D file;
14.
15.   return 0;
16. }

Both functions are assigned to the same struct type and field.
1. static const struct file_operations kprobe_profile_ops =3D {
2. =E2=80=A6
3.   .open           =3D profile_open,
4. =E2=80=A6
5. };
6. static const struct file_operations ftrace_event_format_fops =3D {
7. =E2=80=A6
8.   .open           =3D trace_format_open,
9. =E2=80=A6
10. };


In conclusion, we assume that these functions work in the same way.
However, it is not appropriate that one function is called without
checking.

Thanks!


Best regards,
Jinmeng Zhou