| Message ID | 20220907125657.12192-8-mateusz.grzonka@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Coly Li |
| Headers | show |
| Series | Mdmonitor refactor and udev event handling improvements | expand |
On 9/7/22 8:56 PM, Mateusz Grzonka wrote: > Also check if autorebuild.pid is a symlink, which we shouldn't accept. > > Signed-off-by: Mateusz Grzonka<mateusz.grzonka@intel.com> Except for the code comment style, the rested part is fine to me. Acked-by: Coly Li <colyli@suse.de> Thanks. Coly Li > --- > Monitor.c | 89 ++++++++++++++++++++++++++++++++++++++----------------- > 1 file changed, 62 insertions(+), 27 deletions(-) > > diff --git a/Monitor.c b/Monitor.c > index 3e09f2a2..aa79cf6c 100644 > --- a/Monitor.c > +++ b/Monitor.c > @@ -32,6 +32,7 @@ > #include <libudev.h> > #endif > > +#define TASK_COMM_LEN 16 > #define EVENT_NAME_MAX 32 > #define AUTOREBUILD_PID_PATH MDMON_DIR "/autorebuild.pid" > > @@ -214,7 +215,7 @@ int Monitor(struct mddev_dev *devlist, > info.hostname[sizeof(info.hostname) - 1] = '\0'; > > if (share){ > - if (check_one_sharer(c->scan)) > + if (check_one_sharer(c->scan) == 2) > return 1; > } > > @@ -394,39 +395,73 @@ static int make_daemon(char *pidfile) > return -1; > } > > +/** > + * check_one_sharer() - Checks for other mdmon processes running. > + * > + * Return: > + * 0 - no other processes running, > + * 1 - warning, > + * 2 - error, or when scan mode is enabled, and one mdmon process already exists > + */ > static int check_one_sharer(int scan) > { > int pid; > - FILE *comm_fp; > - FILE *fp; > + FILE *fp, *comm_fp; > char comm_path[PATH_MAX]; > - char path[PATH_MAX]; > - char comm[20]; > - > - sprintf(path, "%s/autorebuild.pid", MDMON_DIR); > - fp = fopen(path, "r"); > - if (fp) { > - if (fscanf(fp, "%d", &pid) != 1) > - pid = -1; > - snprintf(comm_path, sizeof(comm_path), > - "/proc/%d/comm", pid); > - comm_fp = fopen(comm_path, "r"); > - if (comm_fp) { > - if (fscanf(comm_fp, "%19s", comm) && > - strncmp(basename(comm), Name, strlen(Name)) == 0) { > - if (scan) { > - pr_err("Only one autorebuild process allowed in scan mode, aborting\n"); > - fclose(comm_fp); > - fclose(fp); > - return 1; > - } else { > - pr_err("Warning: One autorebuild process already running.\n"); > - } > - } > + char comm[TASK_COMM_LEN]; > + > + if (!is_directory(MDMON_DIR)) { > + pr_err("%s is not a regular directory.\n", MDMON_DIR); > + return 2; > + } > + > + if (access(AUTOREBUILD_PID_PATH, F_OK) != 0) > + return 0; > + > + if (!is_file(AUTOREBUILD_PID_PATH)) { > + pr_err("%s is not a regular file.\n", AUTOREBUILD_PID_PATH); > + return 2; > + } > + > + fp = fopen(AUTOREBUILD_PID_PATH, "r"); > + if (!fp) { > + pr_err("Cannot open %s file.\n", AUTOREBUILD_PID_PATH); > + return 2; > + } > + > + if (fscanf(fp, "%d", &pid) != 1) { > + pr_err("Cannot read pid from %s file.\n", AUTOREBUILD_PID_PATH); > + fclose(fp); > + return 2; > + } > + > + snprintf(comm_path, sizeof(comm_path), "/proc/%d/comm", pid); > + > + comm_fp = fopen(comm_path, "r"); > + if (!comm_fp) { > + dprintf("Warning: Cannot open %s, continuing\n", comm_path); > + fclose(fp); > + return 1; > + } > + > + if (fscanf(comm_fp, "%15s", comm) == 0) { > + dprintf("Warning: Cannot read comm from %s, continuing\n", comm_path); > + fclose(comm_fp); > + fclose(fp); > + return 1; > + } > + > + if (strncmp(basename(comm), Name, strlen(Name)) == 0) { > + if (scan) { > + pr_err("Only one autorebuild process allowed in scan mode, aborting\n"); > fclose(comm_fp); > + fclose(fp); > + return 2; > } > - fclose(fp); > + pr_err("Warning: One autorebuild process already running.\n"); > } > + fclose(comm_fp); > + fclose(fp); > return 0; > } >
diff --git a/Monitor.c b/Monitor.c index 3e09f2a2..aa79cf6c 100644 --- a/Monitor.c +++ b/Monitor.c @@ -32,6 +32,7 @@ #include <libudev.h> #endif +#define TASK_COMM_LEN 16 #define EVENT_NAME_MAX 32 #define AUTOREBUILD_PID_PATH MDMON_DIR "/autorebuild.pid" @@ -214,7 +215,7 @@ int Monitor(struct mddev_dev *devlist, info.hostname[sizeof(info.hostname) - 1] = '\0'; if (share){ - if (check_one_sharer(c->scan)) + if (check_one_sharer(c->scan) == 2) return 1; } @@ -394,39 +395,73 @@ static int make_daemon(char *pidfile) return -1; } +/** + * check_one_sharer() - Checks for other mdmon processes running. + * + * Return: + * 0 - no other processes running, + * 1 - warning, + * 2 - error, or when scan mode is enabled, and one mdmon process already exists + */ static int check_one_sharer(int scan) { int pid; - FILE *comm_fp; - FILE *fp; + FILE *fp, *comm_fp; char comm_path[PATH_MAX]; - char path[PATH_MAX]; - char comm[20]; - - sprintf(path, "%s/autorebuild.pid", MDMON_DIR); - fp = fopen(path, "r"); - if (fp) { - if (fscanf(fp, "%d", &pid) != 1) - pid = -1; - snprintf(comm_path, sizeof(comm_path), - "/proc/%d/comm", pid); - comm_fp = fopen(comm_path, "r"); - if (comm_fp) { - if (fscanf(comm_fp, "%19s", comm) && - strncmp(basename(comm), Name, strlen(Name)) == 0) { - if (scan) { - pr_err("Only one autorebuild process allowed in scan mode, aborting\n"); - fclose(comm_fp); - fclose(fp); - return 1; - } else { - pr_err("Warning: One autorebuild process already running.\n"); - } - } + char comm[TASK_COMM_LEN]; + + if (!is_directory(MDMON_DIR)) { + pr_err("%s is not a regular directory.\n", MDMON_DIR); + return 2; + } + + if (access(AUTOREBUILD_PID_PATH, F_OK) != 0) + return 0; + + if (!is_file(AUTOREBUILD_PID_PATH)) { + pr_err("%s is not a regular file.\n", AUTOREBUILD_PID_PATH); + return 2; + } + + fp = fopen(AUTOREBUILD_PID_PATH, "r"); + if (!fp) { + pr_err("Cannot open %s file.\n", AUTOREBUILD_PID_PATH); + return 2; + } + + if (fscanf(fp, "%d", &pid) != 1) { + pr_err("Cannot read pid from %s file.\n", AUTOREBUILD_PID_PATH); + fclose(fp); + return 2; + } + + snprintf(comm_path, sizeof(comm_path), "/proc/%d/comm", pid); + + comm_fp = fopen(comm_path, "r"); + if (!comm_fp) { + dprintf("Warning: Cannot open %s, continuing\n", comm_path); + fclose(fp); + return 1; + } + + if (fscanf(comm_fp, "%15s", comm) == 0) { + dprintf("Warning: Cannot read comm from %s, continuing\n", comm_path); + fclose(comm_fp); + fclose(fp); + return 1; + } + + if (strncmp(basename(comm), Name, strlen(Name)) == 0) { + if (scan) { + pr_err("Only one autorebuild process allowed in scan mode, aborting\n"); fclose(comm_fp); + fclose(fp); + return 2; } - fclose(fp); + pr_err("Warning: One autorebuild process already running.\n"); } + fclose(comm_fp); + fclose(fp); return 0; }
Also check if autorebuild.pid is a symlink, which we shouldn't accept. Signed-off-by: Mateusz Grzonka <mateusz.grzonka@intel.com> --- Monitor.c | 89 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 62 insertions(+), 27 deletions(-)