[PATCH RFC 1/3] fs: add open O_TMPFILE support

[PATCH RFC 1/3] fs: add open O_TMPFILE support

[Ahmad Fatoum]

barebox dentry cache is never cleared with the assumption that there
should be enough RAM anyway to cache all lookups until boot.

When fuzzing barebox however, there is no limit to how many dentries
are added to the cache. This is e.g. problematic when fuzzing the FIT
parser: FIT images can have compressed payloads. Compressed payloads are
passed to uncompress_buf_to_buf, which uses a new random file in ramfs
as destination. A fuzzer would thus create a dentry for every iteration,
rapidly depleting memory.

A general solution for that would be dropping the dentry cache on memory
pressure. In the special case of uncompress_buf_to_buf, it would already
be enough though to sidestep the dentry cache and create an anonymous
file. Linux provides this with the O_TMPFILE option, so let's add the
equivalent to barebox.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/fs.c         | 29 +++++++++++++++++++++++++++++
 include/fcntl.h |  3 +++
 2 files changed, 32 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 1800d6826ddc..6bd3c2df3c31 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -2539,6 +2539,35 @@ int open(const char *pathname, int flags, ...)
 	const char *s;
 	struct filename *filename;
 
+	if (flags & O_TMPFILE) {
+		fsdev = get_fsdevice_by_path(pathname);
+		if (!fsdev) {
+			errno = ENOENT;
+			return -errno;
+		}
+
+		if (fsdrv != ramfs_driver) {
+			errno = EOPNOTSUPP;
+			return -errno;
+		}
+
+		f = get_file();
+		if (!f) {
+			errno = EMFILE;
+			return -errno;
+		}
+
+		f->path = NULL;
+		f->dentry = NULL;
+		f->f_inode = new_inode(&fsdev->sb);
+		f->f_inode->i_mode = S_IFREG;
+		f->flags = flags;
+		f->size = 0;
+		f->fsdev = fsdev;
+
+		return f->no;
+	}
+
 	filename = getname(pathname);
 	if (IS_ERR(filename))
 		return PTR_ERR(filename);
diff --git a/include/fcntl.h b/include/fcntl.h
index 2e7c0eed3479..1b4cd8ad3783 100644
--- a/include/fcntl.h
+++ b/include/fcntl.h
@@ -16,6 +16,9 @@
 #define O_APPEND	00002000
 #define O_DIRECTORY	00200000	/* must be a directory */
 #define O_NOFOLLOW	00400000	/* don't follow links */
+#define __O_TMPFILE	020000000
+
+#define O_TMPFILE       (__O_TMPFILE | O_DIRECTORY)
 
 /* barebox additional flags */
 #define O_RWSIZE_MASK	017000000
-- 
2.39.2

[PATCH RFC 2/3] libfile: implement read_fd counterpart to read_file

[Ahmad Fatoum]

Files opened with O_TMPFILE have no name, so read_file can't be used
with them. Therefore add a read_fd function, which slurps all a file's
contents into a buffer.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/libfile.h |  2 ++
 lib/libfile.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/include/libfile.h b/include/libfile.h
index a353ccfa9ea9..423e7ffec5b7 100644
--- a/include/libfile.h
+++ b/include/libfile.h
@@ -12,6 +12,8 @@ char *read_file_line(const char *fmt, ...);
 
 void *read_file(const char *filename, size_t *size);
 
+void *read_fd(int fd, size_t *size);
+
 int read_file_2(const char *filename, size_t *size, void **outbuf,
 		loff_t max_size);
 
diff --git a/lib/libfile.c b/lib/libfile.c
index e53ba08415a2..c257baaa2733 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -306,6 +306,50 @@ void *read_file(const char *filename, size_t *size)
 }
 EXPORT_SYMBOL(read_file);
 
+/**
+ * read_fd - read from a file descriptor to an allocated buffer
+ * @filename:  The file descriptor to read
+ * @size:      After successful return contains the size of the file
+ *
+ * This function reads a file descriptor from offset 0 until EOF to an
+ * allocated buffer.
+ *
+ * Return: The buffer containing the file or NULL on failure
+ */
+void *read_fd(int fd, size_t *out_size)
+{
+	off_t off;
+	ssize_t ret;
+	size_t size;
+	void *buf;
+
+	off = lseek(fd, SEEK_CUR, 0);
+	if (off >= 0) {
+		size = off;
+		off = lseek(fd, SEEK_SET, 0);
+	}
+	if (off < 0) {
+		ret = off;
+		goto close_fd;
+	}
+
+	buf = malloc(size + 3);
+	ret = read_full(fd, buf, size);
+	if (ret < 0) {
+		free(buf);
+		goto close_fd;
+	}
+
+	memset(&buf[size], '\0', 3);
+	*out_size = size;
+
+close_fd:
+	close(fd);
+
+	return ret < 0 ? NULL : buf;
+}
+EXPORT_SYMBOL(read_fd);
+
 /**
  * write_file - write a buffer to a file
  * @filename:    The filename to write
-- 
2.39.2

[PATCH RFC 3/3] uncompress: skip dentry cache in uncompress_buf_to_buf

[Ahmad Fatoum]

make_temp() creates a named temporary file, which even after deletion
will keep a negative dentry cache entry that's never freed.

As we don't use the file name for anything, we can just get our
temporary file via open(O_TMPFILE), which won't involve the dentry cache
at all and thereby avoiding leaking memory when fuzzing uncompress_buf_to_buf.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/uncompress.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/lib/uncompress.c b/lib/uncompress.c
index 71ac882b87fe..bfe042fcf83e 100644
--- a/lib/uncompress.c
+++ b/lib/uncompress.c
@@ -185,30 +185,26 @@ int uncompress_buf_to_fd(const void *input, size_t input_len,
 ssize_t uncompress_buf_to_buf(const void *input, size_t input_len,
 			      void **buf, void(*error_fn)(char *x))
 {
-	char *dstpath;
 	size_t size;
-	int outfd, ret;
+	int fd, ret;
+	void *p;
 
-	dstpath = make_temp("data-uncompressed");
-	if (!dstpath)
-		return -ENOMEM;
+	fd = open("/tmp", O_TMPFILE | O_RDWR);
+	if (fd < 0)
+		return -ENODEV;
 
-	outfd = open(dstpath, O_CREAT | O_WRONLY);
-	if (outfd < 0) {
-		ret = -ENODEV;
-		goto free_temp;
-	}
-
-	ret = uncompress_buf_to_fd(input, input_len, outfd, error_fn);
+	ret = uncompress_buf_to_fd(input, input_len, fd, error_fn);
 	if (ret)
-		goto close_outfd;
+		goto close_fd;
 
-	*buf = read_file(dstpath, &size);
-close_outfd:
-	close(outfd);
-	unlink(dstpath);
-free_temp:
-	free(dstpath);
+	p = read_fd(fd, &size);
+	if (p)
+		*buf = p;
+	else
+		ret = -errno;
+
+close_fd:
+	close(fd);
 
 	return ret ?: size;
 }
-- 
2.39.2

Re: [PATCH RFC 2/3] libfile: implement read_fd counterpart to read_file

[Yann Sionneau]

Hello Ahmad,

On 11/20/23 09:37, Ahmad Fatoum wrote:
> Files opened with O_TMPFILE have no name, so read_file can't be used
> with them. Therefore add a read_fd function, which slurps all a file's
> contents into a buffer.
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>   include/libfile.h |  2 ++
>   lib/libfile.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 46 insertions(+)
>
> diff --git a/include/libfile.h b/include/libfile.h
> index a353ccfa9ea9..423e7ffec5b7 100644
> --- a/include/libfile.h
> +++ b/include/libfile.h
> @@ -12,6 +12,8 @@ char *read_file_line(const char *fmt, ...);
>   
>   void *read_file(const char *filename, size_t *size);
>   
> +void *read_fd(int fd, size_t *size);
> +
>   int read_file_2(const char *filename, size_t *size, void **outbuf,
>   		loff_t max_size);
>   
> diff --git a/lib/libfile.c b/lib/libfile.c
> index e53ba08415a2..c257baaa2733 100644
> --- a/lib/libfile.c
> +++ b/lib/libfile.c
> @@ -306,6 +306,50 @@ void *read_file(const char *filename, size_t *size)
>   }
>   EXPORT_SYMBOL(read_file);
>   
> +/**
> + * read_fd - read from a file descriptor to an allocated buffer
> + * @filename:  The file descriptor to read
> + * @size:      After successful return contains the size of the file
> + *
> + * This function reads a file descriptor from offset 0 until EOF to an
> + * allocated buffer.
> + *
> + * Return: The buffer containing the file or NULL on failure
> + */
> +void *read_fd(int fd, size_t *out_size)
> +{
> +	off_t off;
> +	ssize_t ret;
> +	size_t size;
> +	void *buf;
> +
> +	off = lseek(fd, SEEK_CUR, 0);
> +	if (off >= 0) {
> +		size = off;
> +		off = lseek(fd, SEEK_SET, 0);
> +	}
> +	if (off < 0) {
> +		ret = off;
> +		goto close_fd;
> +	}
> +
> +	buf = malloc(size + 3);

Maybe check the return value from malloc()?

Also, it's not clear to me why this +3 is needed and the 3 extra \0, 
maybe an in-code comment would help?

> +	ret = read_full(fd, buf, size);
> +	if (ret < 0) {
> +		free(buf);
> +		goto close_fd;
> +	}
> +
> +	memset(&buf[size], '\0', 3);
> +	*out_size = size;
> +
> +close_fd:
> +	close(fd);
> +
> +	return ret < 0 ? NULL : buf;
> +}
> +EXPORT_SYMBOL(read_fd);
> +
>   /**
>    * write_file - write a buffer to a file
>    * @filename:    The filename to write

Regards,

-- 

Yann

[PATCH] fixup! libfile: implement read_fd counterpart to read_file

[Ahmad Fatoum]

- Justify why the buffer is three bytes longer than the file's content
  (Yann)
- Check for malloc failure (Yann)
- Don't close file descriptor on failure as it's opened outside
  the function
- set errno to indicate why the function failed

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 lib/libfile.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/lib/libfile.c b/lib/libfile.c
index c257baaa2733..a7762ab833f6 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -314,7 +314,9 @@ EXPORT_SYMBOL(read_file);
  * This function reads a file descriptor from offset 0 until EOF to an
  * allocated buffer.
  *
- * Return: The buffer containing the file or NULL on failure
+ * Return: On success, returns a nul-terminated buffer with the file's
+ * contents that should be deallocated with free().
+ * On error, NULL is returned and errno is set to an error code.
  */
 void *read_fd(int fd, size_t *out_size)
 {
@@ -329,24 +331,33 @@ void *read_fd(int fd, size_t *out_size)
 		off = lseek(fd, SEEK_SET, 0);
 	}
 	if (off < 0) {
-		ret = off;
-		goto close_fd;
+		errno = -off;
+		return NULL;
 	}
 
+	/* For user convenience, we always nul-terminate the buffer in
+	 * case it contains a string. As we don't want to assume the string
+	 * to be either an array of char or wchar_t, we just unconditionally
+	 * add 2 bytes as terminator. As the two byte terminator needs to be
+	 * aligned, we just make it three bytes
+	 */
 	buf = malloc(size + 3);
+	if (!buf) {
+		errno = ENOMEM;
+		return NULL;
+	}
+
 	ret = read_full(fd, buf, size);
 	if (ret < 0) {
 		free(buf);
-		goto close_fd;
+		errno = -ret;
+		return NULL;
 	}
 
-	memset(&buf[size], '\0', 3);
+	memset(buf + size, '\0', 3);
 	*out_size = size;
 
-close_fd:
-	close(fd);
-
-	return ret < 0 ? NULL : buf;
+	return buf;
 }
 EXPORT_SYMBOL(read_fd);
 
-- 
2.39.2

Re: [PATCH RFC 2/3] libfile: implement read_fd counterpart to read_file

[Ahmad Fatoum]

Hello Yann,

On 20.11.23 10:49, Yann Sionneau wrote:
> Hello Ahmad,
> 
> On 11/20/23 09:37, Ahmad Fatoum wrote:
>> Files opened with O_TMPFILE have no name, so read_file can't be used
>> with them. Therefore add a read_fd function, which slurps all a file's
>> contents into a buffer.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>   include/libfile.h |  2 ++
>>   lib/libfile.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 46 insertions(+)
>>
>> diff --git a/include/libfile.h b/include/libfile.h
>> index a353ccfa9ea9..423e7ffec5b7 100644
>> --- a/include/libfile.h
>> +++ b/include/libfile.h
>> @@ -12,6 +12,8 @@ char *read_file_line(const char *fmt, ...);
>>     void *read_file(const char *filename, size_t *size);
>>   +void *read_fd(int fd, size_t *size);
>> +
>>   int read_file_2(const char *filename, size_t *size, void **outbuf,
>>           loff_t max_size);
>>   diff --git a/lib/libfile.c b/lib/libfile.c
>> index e53ba08415a2..c257baaa2733 100644
>> --- a/lib/libfile.c
>> +++ b/lib/libfile.c
>> @@ -306,6 +306,50 @@ void *read_file(const char *filename, size_t *size)
>>   }
>>   EXPORT_SYMBOL(read_file);
>>   +/**
>> + * read_fd - read from a file descriptor to an allocated buffer
>> + * @filename:  The file descriptor to read
>> + * @size:      After successful return contains the size of the file
>> + *
>> + * This function reads a file descriptor from offset 0 until EOF to an
>> + * allocated buffer.
>> + *
>> + * Return: The buffer containing the file or NULL on failure
>> + */
>> +void *read_fd(int fd, size_t *out_size)
>> +{
>> +    off_t off;
>> +    ssize_t ret;
>> +    size_t size;
>> +    void *buf;
>> +
>> +    off = lseek(fd, SEEK_CUR, 0);
>> +    if (off >= 0) {
>> +        size = off;
>> +        off = lseek(fd, SEEK_SET, 0);
>> +    }
>> +    if (off < 0) {
>> +        ret = off;
>> +        goto close_fd;
>> +    }
>> +
>> +    buf = malloc(size + 3);
> 
> Maybe check the return value from malloc()?

Yes, I should.

> 
> Also, it's not clear to me why this +3 is needed and the 3 extra \0, maybe an in-code comment would help?

I added a comment in the fixup that I just sent.

Thanks for the review,
Ahmad

> 
>> +    ret = read_full(fd, buf, size);
>> +    if (ret < 0) {
>> +        free(buf);
>> +        goto close_fd;
>> +    }
>> +
>> +    memset(&buf[size], '\0', 3);
>> +    *out_size = size;
>> +
>> +close_fd:
>> +    close(fd);
>> +
>> +    return ret < 0 ? NULL : buf;
>> +}
>> +EXPORT_SYMBOL(read_fd);
>> +
>>   /**
>>    * write_file - write a buffer to a file
>>    * @filename:    The filename to write
> 
> Regards,
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH RFC 1/3] fs: add open O_TMPFILE support

[Ahmad Fatoum]

On 20.11.23 09:37, Ahmad Fatoum wrote:
> barebox dentry cache is never cleared with the assumption that there
> should be enough RAM anyway to cache all lookups until boot.
> 
> When fuzzing barebox however, there is no limit to how many dentries
> are added to the cache. This is e.g. problematic when fuzzing the FIT
> parser: FIT images can have compressed payloads. Compressed payloads are
> passed to uncompress_buf_to_buf, which uses a new random file in ramfs
> as destination. A fuzzer would thus create a dentry for every iteration,
> rapidly depleting memory.
> 
> A general solution for that would be dropping the dentry cache on memory
> pressure. In the special case of uncompress_buf_to_buf, it would already
> be enough though to sidestep the dentry cache and create an anonymous
> file. Linux provides this with the O_TMPFILE option, so let's add the
> equivalent to barebox.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  fs/fs.c         | 29 +++++++++++++++++++++++++++++
>  include/fcntl.h |  3 +++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/fs/fs.c b/fs/fs.c
> index 1800d6826ddc..6bd3c2df3c31 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -2539,6 +2539,35 @@ int open(const char *pathname, int flags, ...)
>  	const char *s;
>  	struct filename *filename;
>  
> +	if (flags & O_TMPFILE) {
> +		fsdev = get_fsdevice_by_path(pathname);
> +		if (!fsdev) {
> +			errno = ENOENT;
> +			return -errno;
> +		}
> +
> +		if (fsdrv != ramfs_driver) {
> +			errno = EOPNOTSUPP;
> +			return -errno;
> +		}

Ouch should be fsdev->driver. Will retest and resend.

> +
> +		f = get_file();
> +		if (!f) {
> +			errno = EMFILE;
> +			return -errno;
> +		}
> +
> +		f->path = NULL;
> +		f->dentry = NULL;
> +		f->f_inode = new_inode(&fsdev->sb);
> +		f->f_inode->i_mode = S_IFREG;
> +		f->flags = flags;
> +		f->size = 0;
> +		f->fsdev = fsdev;
> +
> +		return f->no;
> +	}
> +
>  	filename = getname(pathname);
>  	if (IS_ERR(filename))
>  		return PTR_ERR(filename);
> diff --git a/include/fcntl.h b/include/fcntl.h
> index 2e7c0eed3479..1b4cd8ad3783 100644
> --- a/include/fcntl.h
> +++ b/include/fcntl.h
> @@ -16,6 +16,9 @@
>  #define O_APPEND	00002000
>  #define O_DIRECTORY	00200000	/* must be a directory */
>  #define O_NOFOLLOW	00400000	/* don't follow links */
> +#define __O_TMPFILE	020000000
> +
> +#define O_TMPFILE       (__O_TMPFILE | O_DIRECTORY)
>  
>  /* barebox additional flags */
>  #define O_RWSIZE_MASK	017000000

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH RFC 2/3] libfile: implement read_fd counterpart to read_file

[Sascha Hauer]

On Mon, Nov 20, 2023 at 09:37:49AM +0100, Ahmad Fatoum wrote:
> Files opened with O_TMPFILE have no name, so read_file can't be used
> with them. Therefore add a read_fd function, which slurps all a file's
> contents into a buffer.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/libfile.h |  2 ++
>  lib/libfile.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/include/libfile.h b/include/libfile.h
> index a353ccfa9ea9..423e7ffec5b7 100644
> --- a/include/libfile.h
> +++ b/include/libfile.h
> @@ -12,6 +12,8 @@ char *read_file_line(const char *fmt, ...);
>  
>  void *read_file(const char *filename, size_t *size);
>  
> +void *read_fd(int fd, size_t *size);
> +
>  int read_file_2(const char *filename, size_t *size, void **outbuf,
>  		loff_t max_size);
>  
> diff --git a/lib/libfile.c b/lib/libfile.c
> index e53ba08415a2..c257baaa2733 100644
> --- a/lib/libfile.c
> +++ b/lib/libfile.c
> @@ -306,6 +306,50 @@ void *read_file(const char *filename, size_t *size)
>  }
>  EXPORT_SYMBOL(read_file);
>  
> +/**
> + * read_fd - read from a file descriptor to an allocated buffer
> + * @filename:  The file descriptor to read
> + * @size:      After successful return contains the size of the file
> + *
> + * This function reads a file descriptor from offset 0 until EOF to an
> + * allocated buffer.
> + *
> + * Return: The buffer containing the file or NULL on failure
> + */
> +void *read_fd(int fd, size_t *out_size)
> +{
> +	off_t off;
> +	ssize_t ret;
> +	size_t size;
> +	void *buf;
> +
> +	off = lseek(fd, SEEK_CUR, 0);

You lseek to the current position. It seems you are trying to determine
the size of the file, so did you mean to use SEEK_END?

You could use fstat() instead. With that you could also correctly handle
FILE_SIZE_STREAM.

> +	if (off >= 0) {
> +		size = off;
> +		off = lseek(fd, SEEK_SET, 0);
> +	}
> +	if (off < 0) {
> +		ret = off;
> +		goto close_fd;
> +	}
> +
> +	buf = malloc(size + 3);
> +	ret = read_full(fd, buf, size);

You can use pread_full() here to avoid having to lseek to the beginning
of the file.

> +	if (ret < 0) {
> +		free(buf);
> +		goto close_fd;
> +	}
> +
> +	memset(&buf[size], '\0', 3);
> +	*out_size = size;
> +
> +close_fd:
> +	close(fd);
> +
> +	return ret < 0 ? NULL : buf;

Return an error pointer or an error integer and pass buf as argument?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH RFC 2/3] libfile: implement read_fd counterpart to read_file

[Ahmad Fatoum]

On 21.11.23 08:36, Sascha Hauer wrote:
> On Mon, Nov 20, 2023 at 09:37:49AM +0100, Ahmad Fatoum wrote:
>> Files opened with O_TMPFILE have no name, so read_file can't be used
>> with them. Therefore add a read_fd function, which slurps all a file's
>> contents into a buffer.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  include/libfile.h |  2 ++
>>  lib/libfile.c     | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 46 insertions(+)
>>
>> diff --git a/include/libfile.h b/include/libfile.h
>> index a353ccfa9ea9..423e7ffec5b7 100644
>> --- a/include/libfile.h
>> +++ b/include/libfile.h
>> @@ -12,6 +12,8 @@ char *read_file_line(const char *fmt, ...);
>>  
>>  void *read_file(const char *filename, size_t *size);
>>  
>> +void *read_fd(int fd, size_t *size);
>> +
>>  int read_file_2(const char *filename, size_t *size, void **outbuf,
>>  		loff_t max_size);
>>  
>> diff --git a/lib/libfile.c b/lib/libfile.c
>> index e53ba08415a2..c257baaa2733 100644
>> --- a/lib/libfile.c
>> +++ b/lib/libfile.c
>> @@ -306,6 +306,50 @@ void *read_file(const char *filename, size_t *size)
>>  }
>>  EXPORT_SYMBOL(read_file);
>>  
>> +/**
>> + * read_fd - read from a file descriptor to an allocated buffer
>> + * @filename:  The file descriptor to read
>> + * @size:      After successful return contains the size of the file
>> + *
>> + * This function reads a file descriptor from offset 0 until EOF to an
>> + * allocated buffer.
>> + *
>> + * Return: The buffer containing the file or NULL on failure
>> + */
>> +void *read_fd(int fd, size_t *out_size)
>> +{
>> +	off_t off;
>> +	ssize_t ret;
>> +	size_t size;
>> +	void *buf;
>> +
>> +	off = lseek(fd, SEEK_CUR, 0);
> 
> You lseek to the current position. It seems you are trying to determine
> the size of the file, so did you mean to use SEEK_END?
> 
> You could use fstat() instead. With that you could also correctly handle
> FILE_SIZE_STREAM.

Will use fstat, thanks.

> 
>> +	if (off >= 0) {
>> +		size = off;
>> +		off = lseek(fd, SEEK_SET, 0);
>> +	}
>> +	if (off < 0) {
>> +		ret = off;
>> +		goto close_fd;
>> +	}
>> +
>> +	buf = malloc(size + 3);
>> +	ret = read_full(fd, buf, size);
> 
> You can use pread_full() here to avoid having to lseek to the beginning
> of the file.

Ok, will do.

> 
>> +	if (ret < 0) {
>> +		free(buf);
>> +		goto close_fd;
>> +	}
>> +
>> +	memset(&buf[size], '\0', 3);
>> +	*out_size = size;
>> +
>> +close_fd:
>> +	close(fd);
>> +
>> +	return ret < 0 ? NULL : buf;
> 
> Return an error pointer or an error integer and pass buf as argument?

I want it to return a buffer like read_file does. I think that would
be the least surprising API.

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |