mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 1/3] fs: add open O_TMPFILE support
@ 2023-11-22 17:03 Ahmad Fatoum
  2023-11-22 17:03 ` [PATCH v2 2/3] libfile: implement read_fd counterpart to read_file Ahmad Fatoum
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2023-11-22 17:03 UTC (permalink / raw)
  To: barebox; +Cc: sha, Yann Sionneau, 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>
---
v1 -> v2:
  - don't use uninitialized fsdrv
---
 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..68e7873e9c54 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 (fsdev->driver != 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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 2/3] libfile: implement read_fd counterpart to read_file
  2023-11-22 17:03 [PATCH v2 1/3] fs: add open O_TMPFILE support Ahmad Fatoum
@ 2023-11-22 17:03 ` Ahmad Fatoum
  2023-11-22 17:03 ` [PATCH v2 3/3] uncompress: skip dentry cache in uncompress_buf_to_buf Ahmad Fatoum
  2023-12-13 13:45 ` [PATCH v2 1/3] fs: add open O_TMPFILE support Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2023-11-22 17:03 UTC (permalink / raw)
  To: barebox; +Cc: sha, Yann Sionneau, 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>
---
v1 -> v2:
  - 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
  - use fstat and pread_full (Sascha)
---
 include/libfile.h |  2 ++
 lib/libfile.c     | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 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..72a2fc79c721 100644
--- a/lib/libfile.c
+++ b/lib/libfile.c
@@ -306,6 +306,56 @@ 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: 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)
+{
+	struct stat st;
+	ssize_t ret;
+	void *buf;
+
+	ret = fstat(fd, &st);
+	if (ret < 0)
+		return NULL;
+
+	if (st.st_size == FILE_SIZE_STREAM) {
+		errno = EINVAL;
+		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(st.st_size + 3);
+	if (!buf)
+		return NULL;
+
+	ret = pread_full(fd, buf, st.st_size, 0);
+	if (ret < 0) {
+		free(buf);
+		return NULL;
+	}
+
+	memset(buf + st.st_size, '\0', 3);
+	*out_size = st.st_size;
+
+	return buf;
+}
+EXPORT_SYMBOL(read_fd);
+
 /**
  * write_file - write a buffer to a file
  * @filename:    The filename to write
-- 
2.39.2




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 3/3] uncompress: skip dentry cache in uncompress_buf_to_buf
  2023-11-22 17:03 [PATCH v2 1/3] fs: add open O_TMPFILE support Ahmad Fatoum
  2023-11-22 17:03 ` [PATCH v2 2/3] libfile: implement read_fd counterpart to read_file Ahmad Fatoum
@ 2023-11-22 17:03 ` Ahmad Fatoum
  2023-12-13 13:45 ` [PATCH v2 1/3] fs: add open O_TMPFILE support Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2023-11-22 17:03 UTC (permalink / raw)
  To: barebox; +Cc: sha, Yann Sionneau, 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




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/3] fs: add open O_TMPFILE support
  2023-11-22 17:03 [PATCH v2 1/3] fs: add open O_TMPFILE support Ahmad Fatoum
  2023-11-22 17:03 ` [PATCH v2 2/3] libfile: implement read_fd counterpart to read_file Ahmad Fatoum
  2023-11-22 17:03 ` [PATCH v2 3/3] uncompress: skip dentry cache in uncompress_buf_to_buf Ahmad Fatoum
@ 2023-12-13 13:45 ` Sascha Hauer
  2 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2023-12-13 13:45 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, Yann Sionneau

On Wed, Nov 22, 2023 at 06:03:21PM +0100, 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>
> ---
> v1 -> v2:
>   - don't use uninitialized fsdrv
> ---
>  fs/fs.c         | 29 +++++++++++++++++++++++++++++
>  include/fcntl.h |  3 +++
>  2 files changed, 32 insertions(+)

Applied, thanks

Sascha

> 
> diff --git a/fs/fs.c b/fs/fs.c
> index 1800d6826ddc..68e7873e9c54 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 (fsdev->driver != 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
> 
> 

-- 
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 |



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-12-13 13:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-22 17:03 [PATCH v2 1/3] fs: add open O_TMPFILE support Ahmad Fatoum
2023-11-22 17:03 ` [PATCH v2 2/3] libfile: implement read_fd counterpart to read_file Ahmad Fatoum
2023-11-22 17:03 ` [PATCH v2 3/3] uncompress: skip dentry cache in uncompress_buf_to_buf Ahmad Fatoum
2023-12-13 13:45 ` [PATCH v2 1/3] fs: add open O_TMPFILE support Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox