[PATCH v2 0/5] s390: add support for extended cmdline length

[PATCH v2 0/5] s390: add support for extended cmdline length

[Sven Schnelle]

Hi,

this patchset adds support for extended command line lengths on s390.
The former limit of 896 has been too short in a some configurations. The
linux kernel now provides an additional field in the parameter area
which contains the maximum allowed command line length. In older kernels
this field is zero. In that case the old limit of 896 bytes is used.
This was introduced with 5ecb2da660ab ("s390: support command lines
longer than 896 bytes") in linux.

And while at it, also add the --reuse-cmdline option and use
KEXEC_ALL_OPTIONS.

Changes in v2:

- add slurp_proc_file() to read proc files without knowing the size upfront
- move command_line string from global to function scope

Sven Schnelle (5):
  s390: add variable command line size
  s390: use KEXEC_ALL_OPTIONS
  add slurp_proc_file()
  use slurp_proc_file() in get_command_line()
  s390: add support for --reuse-cmdline

 kexec/arch/s390/crashdump-s390.c       |  3 +-
 kexec/arch/s390/include/arch/options.h | 10 +--
 kexec/arch/s390/kexec-image.c          | 84 ++++++++++++++++----------
 kexec/arch/s390/kexec-s390.h           | 21 ++++---
 kexec/kexec.c                          | 58 +++++++++++++-----
 5 files changed, 114 insertions(+), 62 deletions(-)

-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 1/5] s390: add variable command line size

[Sven Schnelle]

Newer s390 kernels support a command line size longer than 896
bytes. Such kernels contain a new member in the parameter area,
which might be utilized by tools like kexec. Older kernels have
the location initialized to zero, so we check whether there's a
non-zero number present and use that. If there isn't, we fallback
to the legacy command line size of 896 bytes.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 kexec/arch/s390/crashdump-s390.c |  3 +-
 kexec/arch/s390/kexec-image.c    | 65 +++++++++++++++++++++-----------
 kexec/arch/s390/kexec-s390.h     | 21 ++++++-----
 3 files changed, 55 insertions(+), 34 deletions(-)

diff --git a/kexec/arch/s390/crashdump-s390.c b/kexec/arch/s390/crashdump-s390.c
index 10f4d607bbcc..3bd9efe6dafe 100644
--- a/kexec/arch/s390/crashdump-s390.c
+++ b/kexec/arch/s390/crashdump-s390.c
@@ -52,7 +52,8 @@ static int create_elf_header(struct kexec_info *info, unsigned long crash_base,
 	elfcorehdr_size = bufsz;
 	snprintf(str, sizeof(str), " elfcorehdr=%ld@%ldK\n",
 		 elfcorehdr_size, elfcorehdr / 1024);
-	command_line_add(str);
+	if (command_line_add(info, str))
+		return -1;
 #endif
 	return 0;
 }
diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index 3c24fdfe3c7c..a52399eafd2a 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -25,7 +25,6 @@
 #include <fcntl.h>
 
 static uint64_t crash_base, crash_end;
-static char command_line[COMMAND_LINESIZE];
 
 static void add_segment_check(struct kexec_info *info, const void *buf,
 			      size_t bufsz, unsigned long base, size_t memsz)
@@ -36,13 +35,18 @@ static void add_segment_check(struct kexec_info *info, const void *buf,
 	add_segment(info, buf, bufsz, crash_base + base, memsz);
 }
 
-int command_line_add(const char *str)
+int command_line_add(struct kexec_info *info, const char *str)
 {
-	if (strlen(command_line) + strlen(str) + 1 > COMMAND_LINESIZE) {
-		fprintf(stderr, "Command line too long.\n");
+	char *tmp = NULL;
+
+	tmp = concat_cmdline(info->command_line, str);
+	if (!tmp) {
+		fprintf(stderr, "out of memory\n");
 		return -1;
 	}
-	strcat(command_line, str);
+
+	free(info->command_line);
+	info->command_line = tmp;
 	return 0;
 }
 
@@ -64,7 +68,7 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 	while ((opt = getopt_long(argc, argv, short_options, options, 0)) != -1) {
 		switch(opt) {
 		case OPT_APPEND:
-			if (command_line_add(optarg))
+			if (command_line_add(info, optarg))
 				return -1;
 			break;
 		case OPT_RAMDISK:
@@ -78,13 +82,16 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 		if (info->initrd_fd == -1) {
 			fprintf(stderr, "Could not open initrd file %s:%s\n",
 					ramdisk, strerror(errno));
+			free(info->command_line);
+			info->command_line = NULL;
 			return -1;
 		}
 	}
 
-	info->command_line = command_line;
-	info->command_line_len = strlen (command_line) + 1;
-
+	if (info->command_line)
+		info->command_line_len = strlen(info->command_line) + 1;
+	else
+		info->command_line_len = 0;
 	return 0;
 }
 
@@ -97,7 +104,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	const char *ramdisk;
 	off_t ramdisk_len;
 	unsigned int ramdisk_origin;
-	int opt;
+	int opt, ret = -1;
 
 	if (info->file_mode)
 		return image_s390_load_file(argc, argv, info);
@@ -112,7 +119,6 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 		};
 	static const char short_options[] = KEXEC_OPT_STR "";
 
-	command_line[0] = 0;
 	ramdisk = NULL;
 	ramdisk_len = 0;
 	ramdisk_origin = 0;
@@ -120,7 +126,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	while ((opt = getopt_long(argc,argv,short_options,options,0)) != -1) {
 		switch(opt) {
 		case OPT_APPEND:
-			if (command_line_add(optarg))
+			if (command_line_add(info, optarg))
 				return -1;
 			break;
 		case OPT_RAMDISK:
@@ -132,7 +138,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		if (parse_iomem_single("Crash kernel\n", &crash_base,
 				       &crash_end))
-			return -1;
+			goto out;
 	}
 
 	/* Add kernel segment */
@@ -151,7 +157,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 		rd_buffer = slurp_file_mmap(ramdisk, &ramdisk_len);
 		if (rd_buffer == NULL) {
 			fprintf(stderr, "Could not read ramdisk.\n");
-			return -1;
+			goto out;
 		}
 		ramdisk_origin = MAX(RAMDISK_ORIGIN_ADDR, kernel_size);
 		ramdisk_origin = _ALIGN_UP(ramdisk_origin, 0x100000);
@@ -160,7 +166,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 	}
 	if (info->kexec_flags & KEXEC_ON_CRASH) {
 		if (load_crashdump_segments(info, crash_base, crash_end))
-			return -1;
+			goto out;
 	} else {
 		info->entry = (void *) IMAGE_READ_OFFSET;
 	}
@@ -183,15 +189,28 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 			*tmp = crash_end - crash_base + 1;
 		}
 	}
-	/*
-	 * We will write a probably given command line.
-	 * First, erase the old area, then setup the new parameters:
-	 */
-	if (strlen(command_line) != 0) {
-		memset(krnl_buffer + COMMAND_LINE_OFFS, 0, COMMAND_LINESIZE);
-		memcpy(krnl_buffer + COMMAND_LINE_OFFS, command_line, strlen(command_line));
+
+	if (info->command_line) {
+		unsigned long maxsize;
+		char *dest = krnl_buffer + COMMAND_LINE_OFFS;
+
+		maxsize = *(unsigned long *)(krnl_buffer + MAX_COMMAND_LINESIZE_OFFS);
+		if (!maxsize)
+			maxsize = LEGACY_COMMAND_LINESIZE;
+
+		if (strlen(info->command_line) > maxsize-1) {
+			fprintf(stderr, "command line too long, maximum allowed size %ld\n",
+				maxsize-1);
+			goto out;
+		}
+		strncpy(dest, info->command_line, maxsize-1);
+		dest[maxsize-1] = '\0';
 	}
-	return 0;
+	ret = 0;
+out:
+	free(info->command_line);
+	info->command_line = NULL;
+	return ret;
 }
 
 int 
diff --git a/kexec/arch/s390/kexec-s390.h b/kexec/arch/s390/kexec-s390.h
index ef53b111e167..6a99518c1c9e 100644
--- a/kexec/arch/s390/kexec-s390.h
+++ b/kexec/arch/s390/kexec-s390.h
@@ -10,16 +10,17 @@
 #ifndef KEXEC_S390_H
 #define KEXEC_S390_H
 
-#define IMAGE_READ_OFFSET     0x10000
+#define IMAGE_READ_OFFSET           0x10000
 
-#define RAMDISK_ORIGIN_ADDR   0x800000
-#define INITRD_START_OFFS     0x408
-#define INITRD_SIZE_OFFS      0x410
-#define OLDMEM_BASE_OFFS      0x418
-#define OLDMEM_SIZE_OFFS      0x420
-#define COMMAND_LINE_OFFS     0x480
-#define COMMAND_LINESIZE      896
-#define MAX_MEMORY_RANGES     1024
+#define RAMDISK_ORIGIN_ADDR         0x800000
+#define INITRD_START_OFFS           0x408
+#define INITRD_SIZE_OFFS            0x410
+#define OLDMEM_BASE_OFFS            0x418
+#define OLDMEM_SIZE_OFFS            0x420
+#define MAX_COMMAND_LINESIZE_OFFS   0x430
+#define COMMAND_LINE_OFFS           0x480
+#define LEGACY_COMMAND_LINESIZE     896
+#define MAX_MEMORY_RANGES           1024
 
 #define MAX(x, y) ((x) > (y) ? (x) : (y))
 #define MIN(x, y) ((x) < (y) ? (x) : (y))
@@ -32,6 +33,6 @@ extern int load_crashdump_segments(struct kexec_info *info,
 				   unsigned long crash_end);
 extern int get_memory_ranges_s390(struct memory_range range[], int *ranges,
 				  int with_crashk);
-extern int command_line_add(const char *str);
+extern int command_line_add(struct kexec_info *info, const char *str);
 
 #endif /* KEXEC_S390_H */
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 2/5] s390: use KEXEC_ALL_OPTIONS

[Sven Schnelle]

KEXEC_ALL_OPTIONS could be used instead defining the same
array several times. This makes code easier to maintain when
new options are added.

Suggested-by: Alexander Egorenkov <egorenar@linux.ibm.com>
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 kexec/arch/s390/kexec-image.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index a52399eafd2a..209ab77ddccb 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -57,10 +57,7 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 
 	static const struct option options[] =
 		{
-			KEXEC_OPTIONS
-			{"command-line",     1, 0, OPT_APPEND},
-			{"append",           1, 0, OPT_APPEND},
-			{"initrd",           1, 0, OPT_RAMDISK},
+			KEXEC_ALL_OPTIONS
 			{0,                  0, 0, 0},
 		};
 	static const char short_options[] = KEXEC_OPT_STR "";
@@ -111,10 +108,7 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 
 	static const struct option options[] =
 		{
-			KEXEC_OPTIONS
-			{"command-line",     1, 0, OPT_APPEND},
-			{"append",           1, 0, OPT_APPEND},
-			{"initrd",           1, 0, OPT_RAMDISK},
+			KEXEC_ALL_OPTIONS
 			{0,                  0, 0, 0},
 		};
 	static const char short_options[] = KEXEC_OPT_STR "";
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 3/5] add slurp_proc_file()

[Sven Schnelle]

slurp_file() cannot be used to read proc files, as they are returning
a size of zero in stat(). Add a function slurp_proc_file() which is
similar to slurp_file(), but doesn't require the size of the file to
be known.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
 kexec/kexec.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index f63b36b771eb..a1acba2adf2a 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1106,6 +1106,38 @@ static void remove_parameter(char *line, const char *param_name)
 	}
 }
 
+static char *slurp_proc_file(const char *filename, size_t *len)
+{
+	ssize_t ret, startpos = 0;
+	unsigned int size = 64;
+	char *buf = NULL, *tmp;
+	int fd;
+
+	fd = open(filename, O_RDONLY);
+	if (fd == -1)
+		return NULL;
+
+	do {
+		size *= 2;
+		tmp = realloc(buf, size);
+		if (!tmp) {
+			free(buf);
+			return NULL;
+		}
+		buf = tmp;
+
+		ret = read(fd, buf + startpos, size - startpos);
+		if (ret < 0) {
+			free(buf);
+			return NULL;
+		}
+		startpos += ret;
+		*len = startpos;
+	} while (ret == size);
+
+	return buf;
+}
+
 /*
  * Returns the contents of the current command line to be used with
  * --reuse-cmdline option.  The function gets called from architecture specific
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 4/5] use slurp_proc_file() in get_command_line()

[Sven Schnelle]

This way the size of the command line that get_command_line() can handle
is no longer fixed.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
 kexec/kexec.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/kexec/kexec.c b/kexec/kexec.c
index a1acba2adf2a..e2c58de48565 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -1153,25 +1153,19 @@ static char *slurp_proc_file(const char *filename, size_t *len)
  */
 char *get_command_line(void)
 {
-	FILE *fp;
-	char *line;
-	const int sizeof_line = 2048;
-
-	line = malloc(sizeof_line);
-	if (line == NULL)
-		die("Could not allocate memory to read /proc/cmdline.");
-
-	fp = fopen("/proc/cmdline", "r");
-	if (!fp)
-		die("Could not open /proc/cmdline.");
-
-	if (fgets(line, sizeof_line, fp) == NULL)
-		die("Can't read /proc/cmdline.");
+	char *p, *line;
+	size_t size;
 
-	fclose(fp);
+	line = slurp_proc_file("/proc/cmdline", &size);
+	if (!line || !size)
+		die("Failed to read /proc/cmdline\n");
 
 	/* strip newline */
-	line[strlen(line) - 1] = '\0';
+	line[size-1] = '\0';
+
+	p = strpbrk(line, "\r\n");
+	if (p)
+		*p = '\0';
 
 	remove_parameter(line, "BOOT_IMAGE");
 	if (kexec_flags & KEXEC_ON_CRASH)
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2 5/5] s390: add support for --reuse-cmdline

[Sven Schnelle]

--reuse-cmdline reads the command line of the currently
running kernel from /proc/cmdline and uses that for the
kernel that should be kexec'd.

Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
Reviewed-by: Alexander Egorenkov <egorenar@linux.ibm.com>
---
 kexec/arch/s390/include/arch/options.h | 10 ++++++----
 kexec/arch/s390/kexec-image.c          |  9 +++++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/kexec/arch/s390/include/arch/options.h b/kexec/arch/s390/include/arch/options.h
index 76044a301ceb..c150244996c7 100644
--- a/kexec/arch/s390/include/arch/options.h
+++ b/kexec/arch/s390/include/arch/options.h
@@ -1,9 +1,10 @@
 #ifndef KEXEC_ARCH_S390_OPTIONS_H
 #define KEXEC_ARCH_S390_OPTIONS_H
 
-#define OPT_ARCH_MAX	(OPT_MAX+0)
-#define OPT_APPEND	OPT_MAX+0
-#define OPT_RAMDISK	OPT_MAX+1
+#define OPT_ARCH_MAX		(OPT_MAX+0)
+#define OPT_APPEND		(OPT_MAX+0)
+#define OPT_RAMDISK		(OPT_MAX+1)
+#define OPT_REUSE_CMDLINE	(OPT_MAX+2)
 
 /* Options relevant to the architecture (excluding loader-specific ones),
  * in this case none:
@@ -31,7 +32,8 @@
 	KEXEC_ARCH_OPTIONS				\
 	{"command-line",     1, 0, OPT_APPEND},		\
 	{"append",           1, 0, OPT_APPEND},		\
-	{"initrd",	     1, 0, OPT_RAMDISK},
+	{"initrd",	     1, 0, OPT_RAMDISK},	\
+	{"reuse-cmdline",    0, 0, OPT_REUSE_CMDLINE },
 
 #define KEXEC_ALL_OPT_STR KEXEC_ARCH_OPT_STR
 
diff --git a/kexec/arch/s390/kexec-image.c b/kexec/arch/s390/kexec-image.c
index 209ab77ddccb..69aaf96812f7 100644
--- a/kexec/arch/s390/kexec-image.c
+++ b/kexec/arch/s390/kexec-image.c
@@ -71,6 +71,10 @@ int image_s390_load_file(int argc, char **argv, struct kexec_info *info)
 		case OPT_RAMDISK:
 			ramdisk = optarg;
 			break;
+		case OPT_REUSE_CMDLINE:
+			free(info->command_line);
+			info->command_line = get_command_line();
+			break;
 		}
 	}
 
@@ -123,6 +127,10 @@ image_s390_load(int argc, char **argv, const char *kernel_buf,
 			if (command_line_add(info, optarg))
 				return -1;
 			break;
+		case OPT_REUSE_CMDLINE:
+			free(info->command_line);
+			info->command_line = get_command_line();
+			break;
 		case OPT_RAMDISK:
 			ramdisk = optarg;
 			break;
@@ -223,5 +231,6 @@ image_s390_usage(void)
 	printf("--command-line=STRING Set the kernel command line to STRING.\n"
 	       "--append=STRING       Set the kernel command line to STRING.\n"
 	       "--initrd=FILENAME     Use the file FILENAME as a ramdisk.\n"
+	       "--reuse-cmdline       Use kernel command line from running system.\n"
 		);
 }
-- 
2.32.0


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 3/5] add slurp_proc_file()

[Philipp Rudo]

Hi Sven,

sorry, i need to nag again.

On Wed, 15 Dec 2021 11:18:34 +0100
Sven Schnelle <svens@linux.ibm.com> wrote:

> slurp_file() cannot be used to read proc files, as they are returning
> a size of zero in stat(). Add a function slurp_proc_file() which is
> similar to slurp_file(), but doesn't require the size of the file to
> be known.
> 
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
>  kexec/kexec.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/kexec/kexec.c b/kexec/kexec.c
> index f63b36b771eb..a1acba2adf2a 100644
> --- a/kexec/kexec.c
> +++ b/kexec/kexec.c
> @@ -1106,6 +1106,38 @@ static void remove_parameter(char *line, const char *param_name)
>  	}
>  }
>  
> +static char *slurp_proc_file(const char *filename, size_t *len)
> +{
> +	ssize_t ret, startpos = 0;
> +	unsigned int size = 64;
> +	char *buf = NULL, *tmp;
> +	int fd;
> +
> +	fd = open(filename, O_RDONLY);
> +	if (fd == -1)
> +		return NULL;
> +
> +	do {
> +		size *= 2;
> +		tmp = realloc(buf, size);
> +		if (!tmp) {
> +			free(buf);
> +			return NULL;
> +		}
> +		buf = tmp;
> +
> +		ret = read(fd, buf + startpos, size - startpos);
> +		if (ret < 0) {
> +			free(buf);
> +			return NULL;
> +		}
> +		startpos += ret;
> +		*len = startpos;
> +	} while (ret == size);

I don't think this will work as intended. 

1) read returns the bytes read. So ret has a maximum value of
   size - startpos and thus can only be equal to size on the first pass
   when startpos = 0.

2) it's not an error when read reads less bytes than requested but can
   happen when, e.g. it get's interrupted.

The simplest solution I see is to simply use 'while (ret)' Even when
that means that there is an extra pass in the loop with an extra
call to realloc.

The cleaner solution probably would be to put the read into a second
loop. I.e. the following should work (no tested)

	do {
		[...]
		do {
			ret = read(fd, buf + startpos, size - startpos);
			if (ret < 0) {
				free(buf);
				return NULL;
			}
			startpos += ret;
		while (ret && startpos != size);

		*len = startpos;
	} while (ret);

Thanks
Philipp


> +
> +	return buf;
> +}
> +
>  /*
>   * Returns the contents of the current command line to be used with
>   * --reuse-cmdline option.  The function gets called from architecture specific


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 3/5] add slurp_proc_file()

[Sven Schnelle]

Philipp Rudo <prudo@redhat.com> writes:

> On Wed, 15 Dec 2021 11:18:34 +0100
> Sven Schnelle <svens@linux.ibm.com> wrote:
>
>> slurp_file() cannot be used to read proc files, as they are returning
>> a size of zero in stat(). Add a function slurp_proc_file() which is
>> similar to slurp_file(), but doesn't require the size of the file to
>> be known.
>> 
>> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
>> ---
>>  kexec/kexec.c | 32 ++++++++++++++++++++++++++++++++
>>  1 file changed, 32 insertions(+)
>> 
>> diff --git a/kexec/kexec.c b/kexec/kexec.c
>> index f63b36b771eb..a1acba2adf2a 100644
>> --- a/kexec/kexec.c
>> +++ b/kexec/kexec.c
>> @@ -1106,6 +1106,38 @@ static void remove_parameter(char *line, const char *param_name)
>>  	}
>>  }
>>  
>> +static char *slurp_proc_file(const char *filename, size_t *len)
>> +{
>> +	ssize_t ret, startpos = 0;
>> +	unsigned int size = 64;
>> +	char *buf = NULL, *tmp;
>> +	int fd;
>> +
>> +	fd = open(filename, O_RDONLY);
>> +	if (fd == -1)
>> +		return NULL;
>> +
>> +	do {
>> +		size *= 2;
>> +		tmp = realloc(buf, size);
>> +		if (!tmp) {
>> +			free(buf);
>> +			return NULL;
>> +		}
>> +		buf = tmp;
>> +
>> +		ret = read(fd, buf + startpos, size - startpos);
>> +		if (ret < 0) {
>> +			free(buf);
>> +			return NULL;
>> +		}
>> +		startpos += ret;
>> +		*len = startpos;
>> +	} while (ret == size);
>
> I don't think this will work as intended. 
>
> 1) read returns the bytes read. So ret has a maximum value of
>    size - startpos and thus can only be equal to size on the first pass
>    when startpos = 0.
>
> 2) it's not an error when read reads less bytes than requested but can
>    happen when, e.g. it get's interrupted.
>
> The simplest solution I see is to simply use 'while (ret)' Even when
> that means that there is an extra pass in the loop with an extra
> call to realloc.
>
> The cleaner solution probably would be to put the read into a second
> loop. I.e. the following should work (no tested)
>
> 	do {
> 		[...]
> 		do {
> 			ret = read(fd, buf + startpos, size - startpos);
> 			if (ret < 0) {
> 				free(buf);
> 				return NULL;
> 			}
> 			startpos += ret;
> 		while (ret && startpos != size);
>
> 		*len = startpos;
> 	} while (ret);
>
> Thanks
> Philipp

Sigh. True. I'll prepare a v3...

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec