2024-02-15 08:44:01

by Matthias Schiffer

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

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/



2024-02-15 08:44:04

by Matthias Schiffer

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

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/


2024-02-15 11:06:33

by bluez.test.bot

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

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

2024-02-20 13:52:18

by patchwork-bot+bluetooth

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

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:

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