NetBSD Problem Report #59247

From www@netbsd.org  Wed Apr  2 17:26:00 2025
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id F1DFA1A9239
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  2 Apr 2025 17:25:59 +0000 (UTC)
Message-Id: <20250402172558.AC5E81A923E@mollari.NetBSD.org>
Date: Wed,  2 Apr 2025 17:25:58 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: pthread_cancelstub.c is inadequately tested
X-Send-Pr-Version: www-1.0

>Number:         59247
>Category:       lib
>Synopsis:       pthread_cancelstub.c is inadequately tested
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 02 17:30:00 +0000 2025
>Last-Modified:  Fri Apr 25 13:10:02 +0000 2025
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The TestCancelD Foundation
>Environment:
>Description:
In my attempt to make accept4() a cancellation point for PR lib/59240: POSIX.1-2024: cancellation point audit, I wrote the wrong signature for the _sys_paccept symbol (the libc symbol that invokes the paccept syscall, which libpthread wraps with cancellation tests).

This led anyone using accept4 to get EFAULT, Bad address, which broke all kinds of applications.

Unfortunately, it did not break any automatic tests.  I wrote new automatic tests for all the cancellation points -- but only for their nature as cancellation points, not for their ordinary functionality.  And while we have automatic tests that call accept4(), they aren't linked against libpthread so we aren't testing the pthread_cancelstub.c logic.
>How-To-Repeat:
1. break a pthread cancel stub
2. watch the test run succeed
3. try to use a real application like python or dbus
>Fix:
Yes, please!

1. Maybe declare _sys_paccept with  typeof(paccept) _sys_paccept;  instead of copying & pasting the signature.
2. Make sure each cancellation stub has an automatic test of its ordinary functionality too.

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/lib/libpthread
Date: Fri, 4 Apr 2025 20:40:58 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Fri Apr  4 20:40:58 UTC 2025

 Modified Files:
 	src/lib/libpthread: pthread_cancelstub.c

 Log Message:
 libpthread: Use typeof rather than copying signatures of cancelstubs.

 This would have caught my previous mistake with paccept.

 Omit needless declarations that are already covered by the
 compat/*/*.h header files.

 No functional change intended.  This is purely an improvement to
 build-time error detection.

 PR lib/59134: POSIX-1.2024: pthread_setcancelstate must be
 async-signal-safe

 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.48 -r1.49 src/lib/libpthread/pthread_cancelstub.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/tests/lib/libpthread
Date: Sat, 5 Apr 2025 11:22:33 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sat Apr  5 11:22:32 UTC 2025

 Modified Files:
 	src/tests/lib/libpthread: Makefile t_cancellation.c
 Added Files:
 	src/tests/lib/libpthread: cancelpoint.h t_compat_cancel.c

 Log Message:
 Add tests for compat functions as cancellation points.

 While here, test kevent too.

 PR lib/59240: POSIX.1-2024: cancellation point audit
 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.17 -r1.18 src/tests/lib/libpthread/Makefile
 cvs rdiff -u -r0 -r1.1 src/tests/lib/libpthread/cancelpoint.h \
     src/tests/lib/libpthread/t_compat_cancel.c
 cvs rdiff -u -r1.3 -r1.4 src/tests/lib/libpthread/t_cancellation.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/distrib/sets/lists
Date: Sat, 5 Apr 2025 13:13:34 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sat Apr  5 13:13:34 UTC 2025

 Modified Files:
 	src/distrib/sets/lists/debug: mi
 	src/distrib/sets/lists/tests: mi

 Log Message:
 t_compat_cancel: Update set lists.

 PR lib/59240: POSIX.1-2024: cancellation point audit
 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.472 -r1.473 src/distrib/sets/lists/debug/mi
 cvs rdiff -u -r1.1364 -r1.1365 src/distrib/sets/lists/tests/mi

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/lib/libc/compat/arch/ia64/sys
Date: Mon, 7 Apr 2025 20:21:11 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Mon Apr  7 20:21:11 UTC 2025

 Modified Files:
 	src/lib/libc/compat/arch/ia64/sys: Makefile.inc
 Added Files:
 	src/lib/libc/compat/arch/ia64/sys: compat_sigprocmask.S
 	    compat_sigsuspend.S

 Log Message:
 ia64: Include compat13 sigprocmask and sigsuspend stubs.

 All the other ports already do this.  Although ia64 was imported long
 after 1.4, the same is true of other ports that nevertheless include
 these stubs, like amd64.

 We could invent a new __NetBSD_Compat_Min__ macro like
 __NetBSD_Version__ that tells the earliest version of NetBSD for
 which we aim to support compat binaries, and use that to
 conditionalize tests like t_compat_cancel.c.  But that's a bit more
 trouble for something we can dispense with by a couple tiny syscall
 stubs.

 Should fix build for:

 PR lib/59240: POSIX.1-2024: cancellation point audit
 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.3 -r1.4 src/lib/libc/compat/arch/ia64/sys/Makefile.inc
 cvs rdiff -u -r0 -r1.1 src/lib/libc/compat/arch/ia64/sys/compat_sigprocmask.S \
     src/lib/libc/compat/arch/ia64/sys/compat_sigsuspend.S

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/tests/lib/libpthread
Date: Tue, 8 Apr 2025 13:30:45 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Tue Apr  8 13:30:45 UTC 2025

 Modified Files:
 	src/tests/lib/libpthread: Makefile

 Log Message:
 tests/lib/libpthread/t_compat_cancel: Make linker warnings non-fatal.

 We will get warnings like:

 /home/riastradh/netbsd/current/src/tests/lib/libpthread/t_compat_cancel.c:233: warning: warning: reference to compatibility sigsuspend(); include <signal.h> for correct reference

 This is intended -- t_compat_cancel deliberately uses the compat
 symbols, not the modern symbols, in order to test the compat symbols.

 Fixes clang build because bsd.sys.mk enables -Wl,--fatal-warnings in
 LDFLAGS by default.

 PR lib/59240: POSIX.1-2024: cancellation point audit
 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.18 -r1.19 src/tests/lib/libpthread/Makefile

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/tests/lib/libpthread
Date: Fri, 11 Apr 2025 02:07:17 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Fri Apr 11 02:07:17 UTC 2025

 Modified Files:
 	src/tests/lib/libpthread: t_compat_cancel.c

 Log Message:
 t_compat_cancel: Fix compat numbers to match reality.

 - msync is as it existed in 1.2, and is implemented by compat12;
   __msync13 was new in 1.3.

 - sigsuspend is as it existed in 1.3, and is implemented by compat13;
   __sigsuspend14 was new in 1.4.

 (But, while select is as it existed in 5.0, and is implemented by
 compat50, __select50 was new in _6.0_, not in 5.0.)

 No functional change intended other than to rename the test cases for
 clarity.

 PR lib/59240: POSIX.1-2024: cancellation point audit
 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.1 -r1.2 src/tests/lib/libpthread/t_compat_cancel.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/59247 CVS commit: src/tests/lib/libpthread
Date: Fri, 25 Apr 2025 13:09:44 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Fri Apr 25 13:09:44 UTC 2025

 Modified Files:
 	src/tests/lib/libpthread: t_compat_cancel.c

 Log Message:
 t_compat_cancel: Use -1 rather than SIGALRM for expected signal.

 On kernels without (e.g.) COMPAT_13, we get SIGSYS rather than
 SIGALRM if the cancellation point doesn't cancel the syscall.

 Won't help if we try to interrupt in-progress waits, but currently
 none of the tests do that; we'll burn that bridge when we come to it,

 PR lib/59240: POSIX.1-2024: cancellation point audit
 PR lib/59247: pthread_cancelstub.c is inadequately tested


 To generate a diff of this commit:
 cvs rdiff -u -r1.2 -r1.3 src/tests/lib/libpthread/t_compat_cancel.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Home	PR Database Search