Re: [PATCH 00/15] sysctl: Remove sentinel elements from drivers

[Joel Granados]

On Thu, Sep 28, 2023 at 04:31:30PM +0000, Christophe Leroy wrote:
> 
> 
> Le 28/09/2023 à 15:21, Joel Granados via B4 Relay a écrit :
> > From: Joel Granados <j.granados@xxxxxxxxxxx>
> 
> Automatic test fails on powerpc, see 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20230928-jag-sysctl_remove_empty_elem_drivers-v1-15-e59120fca9f9@xxxxxxxxxxx/
From this I got to this URL
https://github.com/linuxppc/linux-snowpatch/actions/runs/6339718136/job/17221399242
and saw this message "sysctl table check failed: dev/tty/ No proc_handler".
This means that we hit the check for entry->proc_handler in
sysctl_check_table.

> 
> Kernel attempted to read user page (1a111316) - exploit attempt? (uid: 0)
> BUG: Unable to handle kernel data access on read at 0x1a111316
> Faulting instruction address: 0xc0545338
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=4K PowerPC 44x Platform
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper Not tainted 6.5.0-rc6-gdef13277bacb #1
> Hardware name: amcc,bamboo 440GR Rev. B 0x422218d3 PowerPC 44x Platform
> NIP:  c0545338 LR: c0548468 CTR: ffffffff
> REGS: c084fae0 TRAP: 0300   Not tainted  (6.5.0-rc6-gdef13277bacb)
> MSR:  00021000 <CE,ME>  CR: 84004288  XER: 00000000
> DEAR: 1a111316 ESR: 00000000
> GPR00: c0548468 c084fbd0 c0888000 c084fc99 00000000 c084fc7c 1a110316 
> 000affff
> GPR08: ffffffff c084fd18 1a111316 04ffffff 22000282 00000000 c00027c0 
> 00000000
> GPR16: 00000000 00000000 c0040000 c003d544 00000001 c003eb2c 096023d4 
> 00000000
> GPR24: c0636502 c0636502 c084fc74 c0588510 c084fc68 c084fc7c c084fc99 
> 00000002
> NIP [c0545338] string+0x78/0x148
> LR [c0548468] vsnprintf+0x3d8/0x824
> Call Trace:
> [c084fbd0] [c084fc7c] 0xc084fc7c (unreliable)
> [c084fbe0] [c0548468] vsnprintf+0x3d8/0x824
> [c084fc30] [c0072dec] vprintk_store+0x17c/0x4c8
> [c084fcc0] [c007322c] vprintk_emit+0xf4/0x2a0
> [c084fd00] [c0073d04] _printk+0x60/0x88
> [c084fd40] [c01ab63c] sysctl_err+0x78/0xa4
> [c084fd80] [c01ab404] __register_sysctl_table+0x6a0/0x6c4
> [c084fde0] [c06a585c] __register_sysctl_init+0x30/0x78
> [c084fe00] [c06a8cc8] tty_init+0x44/0x168
> [c084fe30] [c00023c4] do_one_initcall+0x64/0x2a0
> [c084fea0] [c068f060] kernel_init_freeable+0x184/0x230
> [c084fee0] [c00027e4] kernel_init+0x24/0x124
> [c084ff00] [c000f1fc] ret_from_kernel_user_thread+0x14/0x1c
I followed this trace and proc_handler is correctly defined in tty_table
(struct ctl_table) in drivers/tty/tty_io.c:tty_init and there is not
path that changes these values.
Additionally, we then fail trying to print instead of continuing with
the initialization. My conjecture is that this might be due to something
different than tht sysctl register call.

Does this happen consistenly or is this just a one off issue?

To what branch are these patches being applied to?

I'm going to post my V2 and keep working on this issue if it pops up
again.

Thx for the report

Best

> --- interrupt: 0 at 0x0
> NIP:  00000000 LR: 00000000 CTR: 00000000
> REGS: c084ff10 TRAP: 0000   Not tainted  (6.5.0-rc6-gdef13277bacb)
> MSR:  00000000 <>  CR: 00000000  XER: 00000000
> 
> GPR00: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> 00000000
> GPR08: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> 00000000
> GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> 00000000
> GPR24: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 
> 00000000
> NIP [00000000] 0x0
> LR [00000000] 0x0
> --- interrupt: 0
> Code: 91610008 90e1000c 4bffd0b5 80010014 38210010 7c0803a6 4e800020 
> 409d0008 99230000 38630001 38840001 4240ffd0 <7d2a20ae> 7f851840 
> 5528063e 2c080000
> ---[ end trace 0000000000000000 ]---
> 
> note: swapper[1] exited with irqs disabled
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> 
> > 
> > What?
> > These commits remove the sentinel element (last empty element) from the
> > sysctl arrays of all the files under the "drivers/" directory that use a
> > sysctl array for registration. The merging of the preparation patches
> > (in https://lore.kernel.org/all/ZO5Yx5JFogGi%2FcBo@xxxxxxxxxxxxxxxxxxxxxx/)
> > to mainline allows us to just remove sentinel elements without changing
> > behavior (more info here [1]).
<--- snip --->
> >   drivers/macintosh/mac_hid.c                   |  3 +-
> >   drivers/md/md.c                               |  3 +-
> >   drivers/misc/sgi-xp/xpc_main.c                |  6 ++--
> >   drivers/net/vrf.c                             |  3 +-
> >   drivers/parport/procfs.c                      | 42 ++++++++++++---------------
> >   drivers/scsi/scsi_sysctl.c                    |  3 +-
> >   drivers/scsi/sg.c                             |  3 +-
> >   drivers/tty/tty_io.c                          |  3 +-
> >   drivers/xen/balloon.c                         |  3 +-
> >   18 files changed, 36 insertions(+), 60 deletions(-)
> > ---
> > base-commit: 0e945134b680040b8613e962f586d91b6d40292d
> > change-id: 20230927-jag-sysctl_remove_empty_elem_drivers-f034962a0d8c
> > 
> > Best regards,

-- 

Joel Granados

Attachment: signature.asc
Description: PGP signature