From 1df6a6ad9a28a8315dad0f1830570b393f8e1eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 1 Apr 2025 16:28:06 +0200 Subject: [PATCH 1/2] simple_cmd executing: show error in case execve fails We don't have to check the return status, since execve only returns on failure. Fixes #52 --- src/executing/simple_cmd/simple_cmd_execute.c | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/executing/simple_cmd/simple_cmd_execute.c b/src/executing/simple_cmd/simple_cmd_execute.c index e483186..6bcba01 100644 --- a/src/executing/simple_cmd/simple_cmd_execute.c +++ b/src/executing/simple_cmd/simple_cmd_execute.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/03/27 16:21:56 by khais #+# #+# */ -/* Updated: 2025/04/01 16:37:31 by khais ### ########.fr */ +/* Updated: 2025/04/01 18:08:02 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -17,6 +17,8 @@ #include "../../env/env_convert.h" #include #include +#include +#include "../../subst/path_split.h" static char **argv_from_wordlist(t_wordlist *wordlist) { @@ -39,6 +41,24 @@ static void command_not_found(t_simple_cmd *cmd) cmd->words->word->word); } +static void ft_execve(char *exe, char **argv, char **envp) +{ + execve(exe, argv, envp); + ft_dprintf(STDERR_FILENO, "minishell: %s: ", argv[0]); + perror(NULL); + free(exe); + path_split_destroy(argv); + path_split_destroy(envp); +} + +static void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app) +{ + ft_execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); + simple_cmd_destroy(cmd); + env_destroy(app->env); + exit(127); +} + void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) { char *exe; @@ -56,7 +76,7 @@ void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) } pid = fork(); if (pid == 0) - execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); + execute_subprocess(exe, cmd, app); free(exe); waitpid(pid, NULL, 0); return ; From e84b88394aa58b5ee4caccf59aea8967fe197119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 1 Apr 2025 18:38:31 +0200 Subject: [PATCH 2/2] fix: double free when getting command path for empty command ``` minishell$ "" ``` leads to the following error: ``` ================================================================= ==1158770==ERROR: AddressSanitizer: attempting double-free on 0x5060000020c0 in thread T0: #0 0x5649fcb5b8a8 in free.part.0 asan_malloc_linux.cpp.o #1 0x5649fcbac549 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:119:3 #2 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #3 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #4 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #5 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #6 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #7 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) #8 0x7fb4f2dcd338 in __libc_start_main@GLIBC_2.2.5 (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a338) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) #9 0x5649fca6f394 in _start (/home/khais/src/42/common_env/minishell/minishell+0x2b394) 0x5060000020c0 is located 0 bytes inside of 58-byte region [0x5060000020c0,0x5060000020fa) freed by thread T0 here: #0 0x5649fcb5b8a8 in free.part.0 asan_malloc_linux.cpp.o #1 0x5649fcbac669 in select_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:81:3 #2 0x5649fcbac524 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:116:9 #3 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #4 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #5 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #6 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #7 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #8 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) previously allocated by thread T0 here: #0 0x5649fcb5c897 in malloc (/home/khais/src/42/common_env/minishell/minishell+0x118897) #1 0x5649fcbac6be in alloc_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp_utils.c:34:9 #2 0x5649fcbac4e9 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:113:15 #3 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #4 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #5 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #6 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #7 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #8 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) SUMMARY: AddressSanitizer: double-free asan_malloc_linux.cpp.o in free.part.0 ==1158770==ABORTING ``` Looks like select_path frees filepath sometimes, no idea why. Removing these lines didn't break any tests. I'm pushing this to get a second opinion --- src/subst/simple_filename_exp.c | 4 +--- test.sh | 7 +++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/subst/simple_filename_exp.c b/src/subst/simple_filename_exp.c index 3e1451b..0f8bd66 100644 --- a/src/subst/simple_filename_exp.c +++ b/src/subst/simple_filename_exp.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/03/02 13:40:10 by jguelen #+# #+# */ -/* Updated: 2025/03/27 18:39:23 by khais ### ########.fr */ +/* Updated: 2025/04/01 18:38:35 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -77,8 +77,6 @@ static char *select_path(char *filepath, char **oldpath, char **path, else return (free(*oldpath), path_split_destroy(path), filepath); } - if (*oldpath != filepath) - free(filepath); return (NULL); } diff --git a/test.sh b/test.sh index e6fcefd..2b647bc 100755 --- a/test.sh +++ b/test.sh @@ -335,6 +335,13 @@ status=1 hello=hi EOF +when_run <