[PATCH BlueZ 1/2] tools/rfcomm: reset ignored signals after fork

[PATCH BlueZ 1/2] tools/rfcomm: reset ignored signals after fork

[Matthias Schiffer]

rfcomm sets SIGCHLD and SIGPIPE to SIG_IGN, which is inherited by child
processes and preserved across execvp(). Many applications do not expect
these signals to be ignored, causing all kinds of breakage (including the
standard C system() function misbehaving on glibc and probably other
libcs because waitpid() does not work when SIGCHLD is ignored).
---
 tools/rfcomm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/rfcomm.c b/tools/rfcomm.c
index e013ff588..f635d4aef 100644
--- a/tools/rfcomm.c
+++ b/tools/rfcomm.c
@@ -212,6 +212,7 @@ static void run_cmdline(struct pollfd *p, sigset_t *sigs, char *devname,
 	int i;
 	pid_t pid;
 	char **cmdargv;
+	struct sigaction sa;
 
 	cmdargv = malloc((argc + 1) * sizeof(char *));
 	if (!cmdargv)
@@ -225,6 +226,11 @@ static void run_cmdline(struct pollfd *p, sigset_t *sigs, char *devname,
 
 	switch (pid) {
 	case 0:
+		memset(&sa, 0, sizeof(sa));
+		sa.sa_handler = SIG_DFL;
+		sigaction(SIGCHLD, &sa, NULL);
+		sigaction(SIGPIPE, &sa, NULL);
+
 		i = execvp(cmdargv[0], cmdargv);
 		fprintf(stderr, "Couldn't execute command %s (errno=%d:%s)\n",
 				cmdargv[0], errno, strerror(errno));
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

[PATCH BlueZ 2/2] tools/rfcomm: _exit() on execvp() failure

[Matthias Schiffer]

When the exec fails, the child process must be terminated instead of
continuing as a second main process.
---
 tools/rfcomm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/rfcomm.c b/tools/rfcomm.c
index f635d4aef..0139fe69b 100644
--- a/tools/rfcomm.c
+++ b/tools/rfcomm.c
@@ -234,7 +234,7 @@ static void run_cmdline(struct pollfd *p, sigset_t *sigs, char *devname,
 		i = execvp(cmdargv[0], cmdargv);
 		fprintf(stderr, "Couldn't execute command %s (errno=%d:%s)\n",
 				cmdargv[0], errno, strerror(errno));
-		break;
+		_exit(EXIT_FAILURE);
 	case -1:
 		fprintf(stderr, "Couldn't fork to execute command %s\n",
 				cmdargv[0]);
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
https://www.tq-group.com/

RE: [BlueZ,1/2] tools/rfcomm: reset ignored signals after fork

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=826253

---Test result---

Test Summary:
CheckPatch                    PASS      0.92 seconds
GitLint                       PASS      0.65 seconds
BuildEll                      PASS      24.66 seconds
BluezMake                     PASS      734.12 seconds
MakeCheck                     PASS      12.14 seconds
MakeDistcheck                 PASS      167.46 seconds
CheckValgrind                 PASS      230.99 seconds
CheckSmatch                   PASS      334.58 seconds
bluezmakeextell               PASS      109.28 seconds
IncrementalBuild              PASS      1378.20 seconds
ScanBuild                     WARNING   985.95 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
tools/rfcomm.c:234:3: warning: Value stored to 'i' is never read
                i = execvp(cmdargv[0], cmdargv);
                ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:234:7: warning: Null pointer passed to 1st parameter expecting 'nonnull'
                i = execvp(cmdargv[0], cmdargv);
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:354:8: warning: Although the value stored to 'fd' is used in the enclosing expression, the value is never actually read from 'fd'
                if ((fd = open(devname, O_RDONLY | O_NOCTTY)) < 0) {
                     ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tools/rfcomm.c:497:14: warning: Assigned value is garbage or undefined
        req.channel = raddr.rc_channel;
                    ^ ~~~~~~~~~~~~~~~~
tools/rfcomm.c:515:8: warning: Although the value stored to 'fd' is used in the enclosing expression, the value is never actually read from 'fd'
                if ((fd = open(devname, O_RDONLY | O_NOCTTY)) < 0) {
                     ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5 warnings generated.



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/2] tools/rfcomm: reset ignored signals after fork

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 15 Feb 2024 09:39:53 +0100 you wrote:
> rfcomm sets SIGCHLD and SIGPIPE to SIG_IGN, which is inherited by child
> processes and preserved across execvp(). Many applications do not expect
> these signals to be ignored, causing all kinds of breakage (including the
> standard C system() function misbehaving on glibc and probably other
> libcs because waitpid() does not work when SIGCHLD is ignored).
> ---
>  tools/rfcomm.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Here is the summary with links:
  - [BlueZ,1/2] tools/rfcomm: reset ignored signals after fork
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=854dcb7c2bbd
  - [BlueZ,2/2] tools/rfcomm: _exit() on execvp() failure
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f79ccf6c429e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html