DKIM-Filter:	OpenDKIM Filter v2.11.0 delorie.com 44QHvb2m234772
X-Authentication-Warning:	delorie.com: mail set sender to djgpp-bounces using -f
DKIM-Filter:	OpenDKIM Filter v2.11.0 delorie.com 44QHvVEn234717
X-Recipient:	djgpp AT delorie DOT com
X-Original-DKIM-Signature:	v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=gmail.com; s=20230601; t=1716746249; x=1717351049; darn=delorie.com;
	h=content-transfer-encoding:in-reply-to:references:to:from
	:content-language:subject:user-agent:mime-version:date:message-id
	:from:to:cc:subject:date:message-id:reply-to;
	bh=4jjqCPE4c/cLghhgjZteTMX71a8mwlIzj3b/oUNOnyU=;
	b=hmnJvVhPTucVv/tUVwjcGB6Yc01YDbRl34uexrHr/MlDpA9k8uq9wLmvmjAzXJtLZk
	p45UlWJIJ7iFx/2uitnuyqpHtcxl2SnLAfN6vvdD3lwDEsn7ixDRFI7MswoSB1TBT7Bh
	Pw4I5Z/WTBkuvcOLt8KQrD3fbPVMX6E510Xj/VnfA2CHntuiLnIqoycyy0EGP/IDdjJV
	eQ00qCnYlpaObHiYSLLVfkP5rCnw3/YLISf0JI3Anx6UzbNB+uEWZlKqbPYAmKV5yvWq
	2vNIL0fiA+HaA2IAMfqMGsXbMXgl7d4XFBzEpSHWWX/EftFsj4+Kbpi6gepE5ANviEZf
	BU/w==
X-Google-DKIM-Signature:	v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=1e100.net; s=20230601; t=1716746249; x=1717351049;
	h=content-transfer-encoding:in-reply-to:references:to:from
	:content-language:subject:user-agent:mime-version:date:message-id
	:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to;
	bh=4jjqCPE4c/cLghhgjZteTMX71a8mwlIzj3b/oUNOnyU=;
	b=j4iHIyS4EIf34cxy0lkObU1N7yqYfDPCc4RjDTWc8fOq/NJLIzw0vGXihgJkvmcIP9
	IdRNnmGzF4h4CWOOHGniSYFwntqfkgF5RMQdYEVjLrhsNCujQNDn3rBtk5dwN1FCuJUI
	5L6ReAUYDNrjMwYLyzXRwSl+xWwW9+PlpqACNlqvwYu1/bRYxYc0Epn2iuVYAfAnmuhp
	tYhzPA+1aRZC4irZCeyEbmzD9In1hP0ooChZm3jrVTXWKgFxRj09lRz6Is3nLH+MKc16
	oDRzEOp53ZvBo3owQkLFQGTMRwMLO4sctArym7wKeh88wj2Z+OpasV5tF52vPF9wzEo1
	avVQ==
X-Gm-Message-State:	AOJu0YwW1ehPQTvgGuVCLZnvOGeDStoTt1+IkPeEiCoF+8nsI7gqevJ1
	g9X+vFUv54IAc+ooq+/RcudrC4JWgVIQEjps+QfRUN4NRuoFxOSumsluzg==
X-Google-Smtp-Source:	AGHT+IEtBhRVmjzEflU3QNyZ8ZLBwmaEs3FGqTnruGzCsCMSuaUugG5viJxcvA1GaNltmjMhnC5GEw==
X-Received:	by 2002:a17:906:a3cc:b0:a59:b17c:c9d2 with SMTP id a640c23a62f3a-a623e8d8909mr831071266b.12.1716746248775;
	Sun, 26 May 2024 10:57:28 -0700 (PDT)
Message-ID:	<55d3008a-a11a-406a-ac3b-2016466384b8@gmail.com>
Date:	Sun, 26 May 2024 19:57:27 +0200
MIME-Version:	1.0
User-Agent:	Mozilla Thunderbird
Subject:	Re: Fix passing fcntl and ioctl variadic arguments to file system
	extension API
From:	"J.W. Jagersma (jwjagersma AT gmail DOT com) [via djgpp AT delorie DOT com]" <djgpp AT delorie DOT com>
To:	djgpp AT delorie DOT com, Pali <pali AT pali DOT im>
References:	<20240509182019 DOT mrbfcyv4jhl3w34x AT pali> <867cg03azy DOT fsf AT gnu DOT org>
	<b97a8b35-4a67-4b75-8e18-45211e7afda7 AT gmail DOT com>
In-Reply-To:	<b97a8b35-4a67-4b75-8e18-45211e7afda7@gmail.com>
Reply-To:	djgpp AT delorie DOT com

On 2024-05-11 17:11, J.W. Jagersma wrote:
> On 2024-05-11 12:21, Eli Zaretskii (eliz AT gnu DOT org) [via djgpp AT delorie DOT com] wrote:
>>> Date: Thu, 9 May 2024 20:20:19 +0200
>>> From: Pali <pali AT pali DOT im>
>>> Cc: djgpp AT delorie DOT com
>>>
>>> Hello. I have there a change which fixes ioctl and fcntl functions when
>>> file descriptor uses file system extension API. These two functions are
>>> takes variadic arguments and file system extension API takes va_list
>>> structure with all corresponding arguments. djgpp code currently do not
>>> pass all arguments to API functions __FSEXT_ioctl and __FSEXT_fcntl.
>>>
>>> My fix, which is below, uses __builtin_apply_args() which returns
>>> pointer to va_list-compatible structure of all arguments (including
>>> non-variadic). It works for djgpp i386 calling convention. It does not
>>> work on e.g. AMD64, but this is not a djgpp case which is 32-bit x86.
>>> For using __builtin_apply_args, it was required to manually inline
>>> __FSEXT_func_wrapper function as it takes variadic arguments and not
>>> va_list structure as the "func" itself.
> 
> I (still) can't see Pali's message but this sounds similar to a patch I
> sent in last year:
> 
> https://www.delorie.com/djgpp/mail-archives/browse.cgi?p=djgpp/2023/06/28/12:14:52
> 
> At the moment, fsext-hooked ioctl() and fnctl() are useless and always
> invoke UB (entirely random effects), so I do think this is a
> high-priority bug.
> 
> Linked patch is technically UB too (sometimes reading past the end of va_args),
> but in that case, there is (should be) no code path where the va_args are used.
> 
> However I wasn't aware of __builtin_apply_args().  Whatever Pali's
> solution looks like, it is probably nicer than mine :)
> 

Getting back to this (and replying to myself)...

From the gcc manual, I noticed there is no mention that
__builtin_apply_args() must return something compatible (or even
similar) to va_list.  It's a neat trick, that may happen to work now on
i386 with stack-based parameters, but there are no guarantees about the
layout of this magic structure.  So, it could break again in the future.

It is really only meant to be used with __builtin_apply(), which also
requires somehow knowing the va_list length beforehand.  And we need to
pass additional arguments to the fsext handler too, so this is just not
practical to use.

Since as far as I can tell, all unix-style fcntl()/ioctl() operations
take at most 1 extra argument (always int/pointer sized), I think my
earlier 15-minute hack may still just be the best solution.  Reading one
element past the end of a va_list is probably not so disastrous - it's
not the sort of "undefined behavior" that the compiler can produce
invalid code for.

- Raw text -