* [PATCH] kexec/kexec.c: Add missing fclose()/close() call
@ 2020-08-25 8:14 Youling Tang
2020-08-25 12:04 ` Eric W. Biederman
2020-08-25 14:50 ` Khalid Aziz and Shuah Khan
0 siblings, 2 replies; 3+ messages in thread
From: Youling Tang @ 2020-08-25 8:14 UTC (permalink / raw)
To: Simon Horman, Eric Biederman, Khalid Aziz,
Hariprasad Nellitheertha, Tim Deegan
Cc: kexec
Add missing fclose()/close() call.
Signed-off-by: Youling Tang <tangyouling@loongson.cn>
---
kexec/kexec.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/kexec/kexec.c b/kexec/kexec.c
index a62b362..f970b13 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -542,6 +542,7 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
}
result = fstat(fd, &stats);
if (result < 0) {
+ close(fd);
die("Cannot stat: %s: %s\n",
filename, strerror(errno));
}
@@ -553,20 +554,26 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
if (S_ISCHR(stats.st_mode)) {
size = lseek(fd, 0, SEEK_END);
- if (size < 0)
+ if (size < 0) {
+ close(fd);
die("Can not seek file %s: %s\n", filename,
strerror(errno));
+ }
err = lseek(fd, 0, SEEK_SET);
- if (err < 0)
+ if (err < 0) {
+ close(fd);
die("Can not seek to the begin of file %s: %s\n",
filename, strerror(errno));
+ }
buf = slurp_fd(fd, filename, size, &nread);
} else if (S_ISBLK(stats.st_mode)) {
err = ioctl(fd, BLKGETSIZE64, &size);
- if (err < 0)
+ if (err < 0) {
+ close(fd);
die("Can't retrieve size of block device %s: %s\n",
filename, strerror(errno));
+ }
buf = slurp_fd(fd, filename, size, &nread);
} else {
size = stats.st_size;
@@ -578,13 +585,18 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
buf = slurp_fd(fd, filename, size, &nread);
}
}
- if ((use_mmap && (buf == MAP_FAILED)) || (!use_mmap && (buf == NULL)))
+ if ((use_mmap && (buf == MAP_FAILED)) || (!use_mmap && (buf == NULL))) {
+ close(fd);
die("Cannot read %s", filename);
+ }
- if (nread != size)
+ if (nread != size) {
+ close(fd);
die("Read on %s ended before stat said it should\n", filename);
+ }
*r_size = size;
+ close(fd);
return buf;
}
@@ -1131,8 +1143,10 @@ char *get_command_line(void)
if (!fp)
die("Could not open /proc/cmdline.");
- if (fgets(line, sizeof_line, fp) == NULL)
+ if (fgets(line, sizeof_line, fp) == NULL) {
+ fclose(fp);
die("Can't read /proc/cmdline.");
+ }
fclose(fp);
@@ -1257,12 +1271,14 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
if (i == file_types) {
fprintf(stderr, "Cannot determine the file type " "of %s\n",
kernel);
+ close(kernel_fd);
return EFAILED;
}
ret = file_type[i].load(argc, argv, kernel_buf, kernel_size, &info);
if (ret < 0) {
fprintf(stderr, "Cannot load %s\n", kernel);
+ close(kernel_fd);
return ret;
}
--
2.1.0
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] kexec/kexec.c: Add missing fclose()/close() call
2020-08-25 8:14 [PATCH] kexec/kexec.c: Add missing fclose()/close() call Youling Tang
@ 2020-08-25 12:04 ` Eric W. Biederman
2020-08-25 14:50 ` Khalid Aziz and Shuah Khan
1 sibling, 0 replies; 3+ messages in thread
From: Eric W. Biederman @ 2020-08-25 12:04 UTC (permalink / raw)
To: Youling Tang
Cc: Khalid Aziz, Simon Horman, kexec, Hariprasad Nellitheertha, Tim Deegan
Youling Tang <tangyouling@loongson.cn> writes:
> Add missing fclose()/close() call.
The program exits immediately and that exit closes all of the files.
What is the point of adding an explicit close?
>
> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> ---
> kexec/kexec.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a62b362..f970b13 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -542,6 +542,7 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
> }
> result = fstat(fd, &stats);
> if (result < 0) {
> + close(fd);
> die("Cannot stat: %s: %s\n",
> filename, strerror(errno));
> }
> @@ -553,20 +554,26 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
> if (S_ISCHR(stats.st_mode)) {
>
> size = lseek(fd, 0, SEEK_END);
> - if (size < 0)
> + if (size < 0) {
> + close(fd);
> die("Can not seek file %s: %s\n", filename,
> strerror(errno));
> + }
>
> err = lseek(fd, 0, SEEK_SET);
> - if (err < 0)
> + if (err < 0) {
> + close(fd);
> die("Can not seek to the begin of file %s: %s\n",
> filename, strerror(errno));
> + }
> buf = slurp_fd(fd, filename, size, &nread);
> } else if (S_ISBLK(stats.st_mode)) {
> err = ioctl(fd, BLKGETSIZE64, &size);
> - if (err < 0)
> + if (err < 0) {
> + close(fd);
> die("Can't retrieve size of block device %s: %s\n",
> filename, strerror(errno));
> + }
> buf = slurp_fd(fd, filename, size, &nread);
> } else {
> size = stats.st_size;
> @@ -578,13 +585,18 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
> buf = slurp_fd(fd, filename, size, &nread);
> }
> }
> - if ((use_mmap && (buf == MAP_FAILED)) || (!use_mmap && (buf == NULL)))
> + if ((use_mmap && (buf == MAP_FAILED)) || (!use_mmap && (buf == NULL))) {
> + close(fd);
> die("Cannot read %s", filename);
> + }
>
> - if (nread != size)
> + if (nread != size) {
> + close(fd);
> die("Read on %s ended before stat said it should\n", filename);
> + }
>
> *r_size = size;
> + close(fd);
> return buf;
> }
>
> @@ -1131,8 +1143,10 @@ char *get_command_line(void)
> if (!fp)
> die("Could not open /proc/cmdline.");
>
> - if (fgets(line, sizeof_line, fp) == NULL)
> + if (fgets(line, sizeof_line, fp) == NULL) {
> + fclose(fp);
> die("Can't read /proc/cmdline.");
> + }
>
> fclose(fp);
>
> @@ -1257,12 +1271,14 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
> if (i == file_types) {
> fprintf(stderr, "Cannot determine the file type " "of %s\n",
> kernel);
> + close(kernel_fd);
> return EFAILED;
> }
>
> ret = file_type[i].load(argc, argv, kernel_buf, kernel_size, &info);
> if (ret < 0) {
> fprintf(stderr, "Cannot load %s\n", kernel);
> + close(kernel_fd);
> return ret;
> }
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] kexec/kexec.c: Add missing fclose()/close() call
2020-08-25 8:14 [PATCH] kexec/kexec.c: Add missing fclose()/close() call Youling Tang
2020-08-25 12:04 ` Eric W. Biederman
@ 2020-08-25 14:50 ` Khalid Aziz and Shuah Khan
1 sibling, 0 replies; 3+ messages in thread
From: Khalid Aziz and Shuah Khan @ 2020-08-25 14:50 UTC (permalink / raw)
To: Youling Tang, Simon Horman, Eric Biederman, Khalid Aziz,
Hariprasad Nellitheertha, Tim Deegan
Cc: kexec
As Eric pointed out, not all of these make sense. I have flagged the
ones that makes sense below.
On 8/25/20 2:14 AM, Youling Tang wrote:
> Add missing fclose()/close() call.
>
> Signed-off-by: Youling Tang <tangyouling@loongson.cn>
> ---
> kexec/kexec.c | 28 ++++++++++++++++++++++------
> 1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index a62b362..f970b13 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -542,6 +542,7 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
> }
> result = fstat(fd, &stats);
> if (result < 0) {
> + close(fd);
> die("Cannot stat: %s: %s\n",
> filename, strerror(errno));
> }
> @@ -553,20 +554,26 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
> if (S_ISCHR(stats.st_mode)) {
>
> size = lseek(fd, 0, SEEK_END);
> - if (size < 0)
> + if (size < 0) {
> + close(fd);
> die("Can not seek file %s: %s\n", filename,
> strerror(errno));
> + }
>
> err = lseek(fd, 0, SEEK_SET);
> - if (err < 0)
> + if (err < 0) {
> + close(fd);
> die("Can not seek to the begin of file %s: %s\n",
> filename, strerror(errno));
> + }
> buf = slurp_fd(fd, filename, size, &nread);
> } else if (S_ISBLK(stats.st_mode)) {
> err = ioctl(fd, BLKGETSIZE64, &size);
> - if (err < 0)
> + if (err < 0) {
> + close(fd);
> die("Can't retrieve size of block device %s: %s\n",
> filename, strerror(errno));
> + }
> buf = slurp_fd(fd, filename, size, &nread);
> } else {
> size = stats.st_size;
> @@ -578,13 +585,18 @@ static char *slurp_file_generic(const char *filename, off_t *r_size,
> buf = slurp_fd(fd, filename, size, &nread);
> }
> }
> - if ((use_mmap && (buf == MAP_FAILED)) || (!use_mmap && (buf == NULL)))
> + if ((use_mmap && (buf == MAP_FAILED)) || (!use_mmap && (buf == NULL))) {
> + close(fd);
> die("Cannot read %s", filename);
> + }
>
> - if (nread != size)
> + if (nread != size) {
> + close(fd);
> die("Read on %s ended before stat said it should\n", filename);
> + }
Above changes do not serve any purpose.
>
> *r_size = size;
> + close(fd);
> return buf;
> }
>
This one is good.
> @@ -1131,8 +1143,10 @@ char *get_command_line(void)
> if (!fp)
> die("Could not open /proc/cmdline.");
>
> - if (fgets(line, sizeof_line, fp) == NULL)
> + if (fgets(line, sizeof_line, fp) == NULL) {
> + fclose(fp);
> die("Can't read /proc/cmdline.");
> + }
>
> fclose(fp);
>
This is unnecessary.
> @@ -1257,12 +1271,14 @@ static int do_kexec_file_load(int fileind, int argc, char **argv,
> if (i == file_types) {
> fprintf(stderr, "Cannot determine the file type " "of %s\n",
> kernel);
> + close(kernel_fd);
> return EFAILED;
> }
>
> ret = file_type[i].load(argc, argv, kernel_buf, kernel_size, &info);
> if (ret < 0) {
> fprintf(stderr, "Cannot load %s\n", kernel);
> + close(kernel_fd);
> return ret;
> }
>
>
These are good.
--
Khalid
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-25 14:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 8:14 [PATCH] kexec/kexec.c: Add missing fclose()/close() call Youling Tang
2020-08-25 12:04 ` Eric W. Biederman
2020-08-25 14:50 ` Khalid Aziz and Shuah Khan
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.