linux-kernel - Re: linux-next: boot warning after merge of the vfs-brauner tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240826154818.hzqnvofdmaxvuwrh@quentin>
Date: Mon, 26 Aug 2024 15:48:18 +0000
From: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>
To: Stephen Rothwell <sfr@...b.auug.org.au>
Cc: Christian Brauner <brauner@...nel.org>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Pankaj Raghav <p.raghav@...sung.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux Next Mailing List <linux-next@...r.kernel.org>,
	djwong@...nel.org, ritesh.list@...il.com,
	linuxppc-dev@...ts.ozlabs.org, christophe.leroy@...roup.eu
Subject: Re: linux-next: boot warning after merge of the vfs-brauner tree

On Mon, Aug 26, 2024 at 05:59:31PM +1000, Stephen Rothwell wrote:
> Hi all,
> 
> After merging the vfs-brauner tree, today's linux-next boot test (powerpc
> pseries_le_defconfig) produced this warning:

iomap dio calls set_memory_ro() on the page that is used for sub block
zeroing.

But looking at powerpc code, they don't support set_memory_ro() for
memory region that belongs to the kernel(LINEAR_MAP_REGION_ID).

/*
 * On hash, the linear mapping is not in the Linux page table so
 * apply_to_existing_page_range() will have no effect. If in the future
 * the set_memory_* functions are used on the linear map this will need
 * to be updated.
 */
if (!radix_enabled()) {
        int region = get_region_id(addr);

        if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
                return -EINVAL;
}

We call set_memory_ro() on the zero page as a extra security measure.
I don't know much about powerpc, but looking at the comment, is it just
adding the following to support it in powerpc:

diff --git a/arch/powerpc/mm/pageattr.c b/arch/powerpc/mm/pageattr.c
index ac22bf28086fa..e6e0b40ba6db4 100644
--- a/arch/powerpc/mm/pageattr.c
+++ b/arch/powerpc/mm/pageattr.c
@@ -94,7 +94,9 @@ int change_memory_attr(unsigned long addr, int numpages, long action)
        if (!radix_enabled()) {
                int region = get_region_id(addr);
 
-               if (WARN_ON_ONCE(region != VMALLOC_REGION_ID && region != IO_REGION_ID))
+               if (WARN_ON_ONCE(region != VMALLOC_REGION_ID &&
+                                region != IO_REGION_ID &&
+                                region != LINEAR_MAP_REGION_ID))
                        return -EINVAL;
        }
 #endif

 If it involves changing more things and this feature will be added to
 powerpc in the future, we could drop the set_memory_ro() call from
 iomap.

 CC: Darrick(as he suggested set_memory_ro() on zero page), Leroy,
 Ritesh, ppc list

> 
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at arch/powerpc/mm/pageattr.c:97 change_memory_attr+0xbc/0x150
> Modules linked in:
> CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.11.0-rc5-06731-g66e0882fba22 #1
> Hardware name: IBM pSeries (emulated by qemu) POWER8 (architected) 0x4d0200 0xf000004 of:SLOF,HEAD pSeries
> NIP:  c00000000008a1ac LR: c00000000008a14c CTR: 0000000000000000
> REGS: c0000000049b7930 TRAP: 0700   Not tainted  (6.11.0-rc5-06731-g66e0882fba22)
> MSR:  8000000002029033 <SF,VEC,EE,ME,IR,DR,RI,LE>  CR: 84000482  XER: 00000000
> CFAR: c00000000008a218 IRQMASK: 0 
> GPR00: c00000000008a14c c0000000049b7bd0 c00000000167b100 0000000000000000 
> GPR04: 0000000000000001 0000000000000000 0000000000000200 c000000002b10878 
> GPR08: 000000007da60000 c007ffffffffffff ffffffffffffffff 0000000084000482 
> GPR12: 0000000000000180 c000000002b90000 c00000000001110c 0000000000000000 
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> GPR20: 0000000000000000 0000000000000000 0000000000000000 c000000001562288 
> GPR24: c000000002003e6c c000000001632418 000000000000018c c0000000020c1058 
> GPR28: 0000000000000000 0000000000000000 c000000006330000 0000000000000001 
> NIP [c00000000008a1ac] change_memory_attr+0xbc/0x150
> LR [c00000000008a14c] change_memory_attr+0x5c/0x150
> Call Trace:
> [c0000000049b7bd0] [000000000000018c] 0x18c (unreliable)
> [c0000000049b7c10] [c00000000206bf70] iomap_dio_init+0x64/0x88
> [c0000000049b7c30] [c000000000010d98] do_one_initcall+0x80/0x2f8
> [c0000000049b7d00] [c000000002005c9c] kernel_init_freeable+0x32c/0x520
> [c0000000049b7de0] [c000000000011138] kernel_init+0x34/0x26c
> [c0000000049b7e50] [c00000000000debc] ret_from_kernel_user_thread+0x14/0x1c
> --- interrupt: 0 at 0x0
> Code: 60000000 e8010050 eba10028 7c6307b4 ebc10030 38210040 ebe1fff8 7c0803a6 4e800020 7bc92720 2c29000c 41820058 <0fe00000> 4800002c 60000000 60000000 
> ---[ end trace 0000000000000000 ]---
> 
> Bisected to commit
> 
>   d940b3b7b76b ("iomap: fix iomap_dio_zero() for fs bs > system page size")
> 
> I have reverted commit
> 
>   9b0ebbc72358 ("Merge patch series "enable bs > ps in XFS"")
> 
> for today.
> 
> -- 
> Cheers,
> Stephen Rothwell



-- 
Pankaj Raghav