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: <f855f1d2-35dd-48bd-8de5-25d850d02d37@linux.ibm.com>
Date: Thu, 23 Oct 2025 18:04:35 +0200
From: Jens Remus <jremus@...ux.ibm.com>
To: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
        bpf@...r.kernel.org, x86@...nel.org, linux-mm@...ck.org,
        Steven Rostedt <rostedt@...nel.org>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andrii Nakryiko <andrii@...nel.org>,
        Indu Bhagat <indu.bhagat@...cle.com>,
        "Jose E. Marchesi" <jemarch@....org>,
        Beau Belgrave <beaub@...ux.microsoft.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Florian Weimer <fweimer@...hat.com>, Kees Cook <kees@...nel.org>,
        "Carlos O'Donell" <codonell@...hat.com>, Sam James <sam@...too.org>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        David Hildenbrand <david@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
        "Liam R. Howlett" <Liam.Howlett@...cle.com>,
        Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
        Michal Hocko
 <mhocko@...e.com>, Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>, Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        "Steven Rostedt (Google)" <rostedt@...dmis.org>
Subject: Re: [PATCH v11 06/15] unwind_user/sframe: Add support for reading
 .sframe contents

Hi Steve, et al.,

as discussed during yesterdays SFrame call I will be sending two RFC
fixup patches shortly as POC to demonstrate how this patch and
"[PATCH v11 14/15] unwind_user/sframe: Add .sframe validation option"
could benefit from introducing an internal FDE representation (e.g.
struct sframe_fde_internal) similar to the used internal FRE
representation (struct sframe_fre).

On 10/22/2025 4:43 PM, Jens Remus wrote:

> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c

> +static __always_inline int __read_fde(struct sframe_section *sec,
> +				      unsigned int fde_num,
> +				      struct sframe_fde *fde,
> +				      unsigned long *fde_start_base)

The goal would be to eliminate the passing through of fde_start_base as
well as the various computations of the effective function start address
(= *fde_start_base + fde->start_addr) throughout this module.  The
internal FDE representation could then simply convey the effective
function start address via an "unsigned long func_start_addr" field.

> +{
> +	unsigned long fde_addr, ip;
> +
> +	fde_addr = sec->fdes_start + (fde_num * sizeof(struct sframe_fde));
> +	unsafe_copy_from_user(fde, (void __user *)fde_addr,
> +			      sizeof(struct sframe_fde), Efault);
> +
> +	ip = fde_addr + fde->start_addr;
> +	if (ip < sec->text_start || ip > sec->text_end)
> +		return -EINVAL;
> +
> +	*fde_start_base = fde_addr;
> +	return 0;
> +
> +Efault:
> +	return -EFAULT;
> +}
> +
> +static __always_inline int __find_fde(struct sframe_section *sec,
> +				      unsigned long ip,
> +				      struct sframe_fde *fde,
> +				      unsigned long *fde_start_base)

fde_start_base would get eliminated.

> +{
> +	unsigned long func_addr_low = 0, func_addr_high = ULONG_MAX;
> +	struct sframe_fde __user *first, *low, *high, *found = NULL;
> +	int ret;
> +
> +	first = (void __user *)sec->fdes_start;
> +	low = first;
> +	high = first + sec->num_fdes - 1;
> +
> +	while (low <= high) {
> +		struct sframe_fde __user *mid;
> +		s32 func_off;
> +		unsigned long func_addr;
> +
> +		mid = low + ((high - low) / 2);
> +
> +		unsafe_get_user(func_off, (s32 __user *)mid, Efault);
> +		func_addr = (unsigned long)mid + func_off;
> +
> +		if (ip >= func_addr) {
> +			if (func_addr < func_addr_low)
> +				return -EFAULT;
> +
> +			func_addr_low = func_addr;
> +
> +			found = mid;
> +			low = mid + 1;
> +		} else {
> +			if (func_addr > func_addr_high)
> +				return -EFAULT;
> +
> +			func_addr_high = func_addr;
> +
> +			high = mid - 1;
> +		}
> +	}
> +
> +	if (!found)
> +		return -EINVAL;
> +
> +	ret = __read_fde(sec, found - first, fde, fde_start_base);

fde_start_base would get eliminated.

> +	if (ret)
> +		return ret;
> +
> +	/* make sure it's not in a gap */
> +	if (ip < *fde_start_base + fde->start_addr ||
> +	    ip >= *fde_start_base + fde->start_addr + fde->func_size)

Would simplify to:

	if (ip < fde->func_start_addr ||
	    ip >= fde->func_start_addr + fde->func_size)

> +		return -EINVAL;
> +
> +	return 0;
> +
> +Efault:
> +	return -EFAULT;
> +}

> +static __always_inline int __find_fre(struct sframe_section *sec,
> +				      struct sframe_fde *fde,
> +				      unsigned long fde_start_base,

fde_start_base would get eliminated.

> +				      unsigned long ip,
> +				      struct unwind_user_frame *frame)
> +{
> +	unsigned char fde_type = SFRAME_FUNC_FDE_TYPE(fde->info);
> +	struct sframe_fre *fre, *prev_fre = NULL;
> +	struct sframe_fre fres[2];
> +	unsigned long fre_addr;
> +	bool which = false;
> +	unsigned int i;
> +	u32 ip_off;
> +
> +	ip_off = ip - (fde_start_base + fde->start_addr);

Would simplify to:

	ip_off = ip - fde->func_start_addr;

> +
> +	if (fde_type == SFRAME_FDE_TYPE_PCMASK)
> +		ip_off %= fde->rep_size;
> +
> +	fre_addr = sec->fres_start + fde->fres_off;
> +
> +	for (i = 0; i < fde->fres_num; i++) {
> +		int ret;
> +
> +		/*
> +		 * Alternate between the two fre_addr[] entries for 'fre' and
> +		 * 'prev_fre'.
> +		 */
> +		fre = which ? fres : fres + 1;
> +		which = !which;
> +
> +		ret = __read_fre(sec, fde, fre_addr, fre);
> +		if (ret)
> +			return ret;
> +
> +		fre_addr += fre->size;
> +
> +		if (prev_fre && fre->ip_off <= prev_fre->ip_off)
> +			return -EFAULT;
> +
> +		if (fre->ip_off > ip_off)
> +			break;
> +
> +		prev_fre = fre;
> +	}
> +
> +	if (!prev_fre)
> +		return -EINVAL;
> +	fre = prev_fre;
> +
> +	frame->cfa_off = fre->cfa_off;
> +	frame->ra_off  = fre->ra_off;
> +	frame->fp_off  = fre->fp_off;
> +	frame->use_fp  = SFRAME_FRE_CFA_BASE_REG_ID(fre->info) == SFRAME_BASE_REG_FP;
> +
> +	return 0;
> +}
> +
> +int sframe_find(unsigned long ip, struct unwind_user_frame *frame)
> +{
> +	struct mm_struct *mm = current->mm;
> +	struct sframe_section *sec;
> +	struct sframe_fde fde;
> +	unsigned long fde_start_base;

fde_start_base would get eliminated.

> +	int ret;
> +
> +	if (!mm)
> +		return -EINVAL;
> +
> +	guard(srcu)(&sframe_srcu);
> +
> +	sec = mtree_load(&mm->sframe_mt, ip);
> +	if (!sec)
> +		return -EINVAL;
> +
> +	if (!user_read_access_begin((void __user *)sec->sframe_start,
> +				    sec->sframe_end - sec->sframe_start))
> +		return -EFAULT;
> +
> +	ret = __find_fde(sec, ip, &fde, &fde_start_base);

fde_start_base would get eliminated.

> +	if (ret)
> +		goto end;
> +
> +	ret = __find_fre(sec, &fde, fde_start_base, ip, frame);

fde_start_base would get eliminated.

> +end:
> +	user_read_access_end();
> +	return ret;
> +}
Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@...ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ