linux-kernel - Re: [PATCH v7 2/6] arm64: ptdump: Expose the attribute parsing functionality

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: <ZppRqmuL2ORp2uNC@google.com>
Date: Fri, 19 Jul 2024 11:44:42 +0000
From: Sebastian Ene <sebastianene@...gle.com>
To: Will Deacon <will@...nel.org>
Cc: akpm@...ux-foundation.org, alexghiti@...osinc.com, ankita@...dia.com,
	ardb@...nel.org, catalin.marinas@....com,
	christophe.leroy@...roup.eu, james.morse@....com,
	vdonnefort@...gle.com, mark.rutland@....com, maz@...nel.org,
	oliver.upton@...ux.dev, rananta@...gle.com, ryan.roberts@....com,
	shahuang@...hat.com, suzuki.poulose@....com, yuzenghui@...wei.com,
	kvmarm@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v7 2/6] arm64: ptdump: Expose the attribute parsing
 functionality

On Fri, Jul 05, 2024 at 12:07:48PM +0100, Will Deacon wrote:
> On Fri, Jun 21, 2024 at 12:32:26PM +0000, Sebastian Ene wrote:
> > Reuse the descriptor parsing functionality to keep the same output format
> > as the original ptdump code. In order for this to happen, move the state
> > tracking objects into a common header.
> > 
> > Signed-off-by: Sebastian Ene <sebastianene@...gle.com>
> > ---
> >  arch/arm64/include/asm/ptdump.h | 41 ++++++++++++++++++++++++++++++++-
> >  arch/arm64/mm/ptdump.c          | 37 ++---------------------------
> >  2 files changed, 42 insertions(+), 36 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
> > index 5b1701c76d1c..c550b2afcab7 100644
> > --- a/arch/arm64/include/asm/ptdump.h
> > +++ b/arch/arm64/include/asm/ptdump.h
> > @@ -9,6 +9,7 @@
> >  
> >  #include <linux/mm_types.h>
> >  #include <linux/seq_file.h>
> > +#include <linux/ptdump.h>
> >  
> >  struct addr_marker {
> >  	unsigned long start_address;
> > @@ -21,14 +22,52 @@ struct ptdump_info {
> >  	unsigned long			base_addr;
> >  };
> >  
> > +struct prot_bits {
> > +	u64		mask;
> > +	u64		val;
> > +	const char	*set;
> > +	const char	*clear;
> > +};
> > +
> > +struct pg_level {
> > +	const struct prot_bits *bits;
> > +	char name[4];
> > +	int num;
> > +	u64 mask;
> > +};
> > +
> > +/*
> > + * The page dumper groups page table entries of the same type into a single
> > + * description. It uses pg_state to track the range information while
> > + * iterating over the pte entries. When the continuity is broken it then
> > + * dumps out a description of the range.
> > + */
> > +struct pg_state {
> > +	struct ptdump_state ptdump;
> > +	struct seq_file *seq;
> > +	const struct addr_marker *marker;
> > +	const struct mm_struct *mm;
> > +	unsigned long start_address;
> > +	int level;
> > +	u64 current_prot;
> > +	bool check_wx;
> > +	unsigned long wx_pages;
> > +	unsigned long uxn_pages;
> > +};

Hello Will,

> 
> Minor nit, but if we're moving these structure definitions into the
> header then I'd be inclined to give them some more specific names (e.g.
> prefix them with 'ptdump_'). Granted, this header isn't used widely, but
> it's included by arch/arm64/mm/mmu.c and claiming 'struct prot_bits' is
> a bit over-reaching imo!
> 
> Will

Thanks for having a look. I applied the prefix.

Cheers,
Seb