mbox series

[0/3] Fix some seq_file users that were recently broken

Message ID 161248518659.21478.2484341937387294998.stgit@noble1 (mailing list archive)
Headers show
Series Fix some seq_file users that were recently broken | expand

Message

NeilBrown Feb. 5, 2021, 12:36 a.m. UTC
A recent change to seq_file broke some users which were using seq_file
in a non-"standard" way ...  though the "standard" isn't documented, so
they can be excused.  The result is a possible leak - of memory in one
case, of references to a 'transport' in the other.

These three patches:
 1/ document and explain the problem
 2/ fix the problem user in x86
 3/ fix the problem user in net/sctp

I suspect the patches should each go through the relevant subsystems,
but I'm including akpm as the original went through him.

Thanks,
NeilBrown

---

NeilBrown (3):
      seq_file: document how per-entry resources are managed.
      x86: fix seq_file iteration for pat/memtype.c
      net: fix iteration for sctp transport seq_files

 Documentation/filesystems/seq_file.rst |  6 ++++++
 arch/x86/mm/pat/memtype.c              |  4 ++--
 net/sctp/proc.c                        | 16 ++++++++++++----
 3 files changed, 20 insertions(+), 6 deletions(-)

--
Signature

Comments

Andrew Morton Feb. 5, 2021, 10:35 p.m. UTC | #1
On Fri, 05 Feb 2021 11:36:30 +1100 NeilBrown <neilb@suse.de> wrote:

> A recent change to seq_file broke some users which were using seq_file
> in a non-"standard" way ...  though the "standard" isn't documented, so
> they can be excused.  The result is a possible leak - of memory in one
> case, of references to a 'transport' in the other.
> 
> These three patches:
>  1/ document and explain the problem
>  2/ fix the problem user in x86
>  3/ fix the problem user in net/sctp
> 

1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
interface") was August 2018, so I don't think "recent" applies here?

I didn't look closely, but it appears that the sctp procfs file is
world-readable.  So we gave unprivileged userspace the ability to leak
kernel memory?

So I'm thinking that we aim for 5.12-rc1 on all three patches with a cc:stable?
Jakub Kicinski Feb. 6, 2021, 10:29 p.m. UTC | #2
On Fri, 5 Feb 2021 14:35:50 -0800 Andrew Morton wrote:
> On Fri, 05 Feb 2021 11:36:30 +1100 NeilBrown <neilb@suse.de> wrote:
> 
> > A recent change to seq_file broke some users which were using seq_file
> > in a non-"standard" way ...  though the "standard" isn't documented, so
> > they can be excused.  The result is a possible leak - of memory in one
> > case, of references to a 'transport' in the other.
> > 
> > These three patches:
> >  1/ document and explain the problem
> >  2/ fix the problem user in x86
> >  3/ fix the problem user in net/sctp
> 
> 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
> interface") was August 2018, so I don't think "recent" applies here?
> 
> I didn't look closely, but it appears that the sctp procfs file is
> world-readable.  So we gave unprivileged userspace the ability to leak
> kernel memory?
> 
> So I'm thinking that we aim for 5.12-rc1 on all three patches with a cc:stable?

I'd rather take the sctp patch sooner, we'll send another batch 
of networking fixes for 5.11, anyway. Would that be okay with you?
Andrew Morton Feb. 7, 2021, 9:11 p.m. UTC | #3
On Sat, 6 Feb 2021 14:29:24 -0800 Jakub Kicinski <kuba@kernel.org> wrote:

> On Fri, 5 Feb 2021 14:35:50 -0800 Andrew Morton wrote:
> > On Fri, 05 Feb 2021 11:36:30 +1100 NeilBrown <neilb@suse.de> wrote:
> > 
> > > A recent change to seq_file broke some users which were using seq_file
> > > in a non-"standard" way ...  though the "standard" isn't documented, so
> > > they can be excused.  The result is a possible leak - of memory in one
> > > case, of references to a 'transport' in the other.
> > > 
> > > These three patches:
> > >  1/ document and explain the problem
> > >  2/ fix the problem user in x86
> > >  3/ fix the problem user in net/sctp
> > 
> > 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
> > interface") was August 2018, so I don't think "recent" applies here?
> > 
> > I didn't look closely, but it appears that the sctp procfs file is
> > world-readable.  So we gave unprivileged userspace the ability to leak
> > kernel memory?
> > 
> > So I'm thinking that we aim for 5.12-rc1 on all three patches with a cc:stable?
> 
> I'd rather take the sctp patch sooner, we'll send another batch 
> of networking fixes for 5.11, anyway. Would that be okay with you?

Sure.
NeilBrown Feb. 7, 2021, 10:45 p.m. UTC | #4
On Fri, Feb 05 2021, Andrew Morton wrote:

> On Fri, 05 Feb 2021 11:36:30 +1100 NeilBrown <neilb@suse.de> wrote:
>
>> A recent change to seq_file broke some users which were using seq_file
>> in a non-"standard" way ...  though the "standard" isn't documented, so
>> they can be excused.  The result is a possible leak - of memory in one
>> case, of references to a 'transport' in the other.
>> 
>> These three patches:
>>  1/ document and explain the problem
>>  2/ fix the problem user in x86
>>  3/ fix the problem user in net/sctp
>> 
>
> 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
> interface") was August 2018, so I don't think "recent" applies here?

I must be getting old :-(

>
> I didn't look closely, but it appears that the sctp procfs file is
> world-readable.  So we gave unprivileged userspace the ability to leak
> kernel memory?

Not quite that bad.  The x86 problem allows arbitrary memory to be
leaked, but that is in debugfs (as I'm sure you saw) so is root-only.
The sctp one only allows an sctp_transport to be pinned.  That is not
good and would, e.g., prevent the module from being unloaded.  But
unlike a simple memory leak it won't affect anything outside of sctp.

>
> So I'm thinking that we aim for 5.12-rc1 on all three patches with a cc:stable?

Thanks for looking after these!

NeilBrown
Jakub Kicinski Feb. 8, 2021, 7:42 p.m. UTC | #5
On Sun, 7 Feb 2021 13:11:45 -0800 Andrew Morton wrote:
> On Sat, 6 Feb 2021 14:29:24 -0800 Jakub Kicinski <kuba@kernel.org> wrote:
> > On Fri, 5 Feb 2021 14:35:50 -0800 Andrew Morton wrote:  
> > > On Fri, 05 Feb 2021 11:36:30 +1100 NeilBrown <neilb@suse.de> wrote:
> > > 1f4aace60b0e ("fs/seq_file.c: simplify seq_file iteration code and
> > > interface") was August 2018, so I don't think "recent" applies here?
> > > 
> > > I didn't look closely, but it appears that the sctp procfs file is
> > > world-readable.  So we gave unprivileged userspace the ability to leak
> > > kernel memory?
> > > 
> > > So I'm thinking that we aim for 5.12-rc1 on all three patches with a cc:stable?  
> > 
> > I'd rather take the sctp patch sooner, we'll send another batch 
> > of networking fixes for 5.11, anyway. Would that be okay with you?  
> 
> Sure.

Applied patch 3 to net, thanks everyone!