| Message ID | 20210319085531.1070347-3-dominique.martinet@atmark-techno.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] utils: move get_output_size from ubivol handler | expand |
Hi Dominique, On 19.03.21 09:55, Dominique Martinet wrote: > copyfile would normally fail with ENOSPC on write, but if we can know > beforehand that the file will not fit it is better to error early and > not disrupt whatever is running. > > The check reuses the existing get_output_size() helper which reads the > decompressed-size or decrypted-size properties depending on the file > type, and is skipped altogether if no size is set (if the file is > uncompressed, the size is always its size in the cpio header) > > No actual check that the decompressed size matches the actual size is > made unless the handler requires it later, and this only checks files > that are not installed directly. > > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > Cc: Christian Storm <christian.storm@siemens.com> > --- > Changelog v1 -> v2: > - use decompressed/decrypted size properties with the ubivol helper > for compatibility > - make freebsd use statfs instead of statvfs; looking at linux man > page linux is better off with statvfs so I kept the linux code identical > to v1. > I downloaded a freeBSD image so I will at least compile test this early > next week, didn't have time to yet -- I've only gone as far as linux > tests (size not set, uncompressed file, compressed file with size set > small enough to fit into partition incorrectly or big enough to be > caught by statvfs) > > > core/stream_interface.c | 2 ++ > core/util.c | 47 +++++++++++++++++++++++++++++++++++++++++ > include/util.h | 1 + > 3 files changed, 50 insertions(+) > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index d459010886b5..574ed47634cb 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -222,6 +222,8 @@ static int extract_files(int fd, struct swupdate_cfg *software) > fdout = openfileoutput(img->extract_file); > if (fdout < 0) > return -1; > + if (!img_check_free_space(img, fdout)) > + return -1; > if (copyfile(fd, &fdout, fdh.size, &offset, 0, 0, 0, &checksum, img->sha256, false, NULL, NULL) < 0) { > close(fdout); > return -1; > diff --git a/core/util.c b/core/util.c > index 0e9ad14d5020..db34d0307699 100644 > --- a/core/util.c > +++ b/core/util.c > @@ -24,6 +24,10 @@ > #include <libgen.h> > #include <regex.h> > > +#if defined(__linux__) > +#include <sys/statvfs.h> > +#endif > + > #include "swupdate.h" > #include "util.h" > #include "generated/autoconf.h" > @@ -951,3 +955,46 @@ long long get_output_size(struct img_type *img, bool warn) > > return bytes; > } > + > +static bool check_free_space(int fd, long long size, char *fname) > +{ > + /* This needs OS-specific implementation because > + * linux's statfs f_bsize is optimal IO size vs. statvfs f_bsize fs block size, > + * and freeBSD is the opposite... > + * As everything else is the same down to field names work around > + * this by just defining an alias > + */ > +#if defined(__FreeBSD__) > +#define statvfs statfs > +#define fstatvfs fstatfs > +#endif Ok > +#if defined(__linux__) || defined(__FreeBSD__) I am also interested which other options are available and if SWUpdate is running on seome other OS.... IMHO you can spare this #ifdef, I do not know about further supported OSes. > + struct statvfs statvfs; > + > + if (fstatvfs(fd, &statvfs)) { > + ERROR("Statfs failed on %s, skipping free space check", fname); > + return true; > + } > + > + if (statvfs.f_bfree * statvfs.f_bsize < size) { > + ERROR("Not enough free space to extract %s (needed %llu, got %lu)", > + fname, size, statvfs.f_bfree * statvfs.f_bsize); > + return false; > + } > +#endif > + > + return true; > +} > + > +bool img_check_free_space(struct img_type *img, int fd) > +{ > + long long size; > + > + size = get_output_size(img, true); > + > + if (size <= 0) > + /* Skip check if no size found */ > + return true; > + > + return check_free_space(fd, size, img->fname); > +} > diff --git a/include/util.h b/include/util.h > index ef487fbed21c..7b5b2a85655d 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -219,6 +219,7 @@ void free_string_array(char **nodes); > int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset, > LOGLEVEL level); > long long get_output_size(struct img_type *img, bool warn); > +bool img_check_free_space(struct img_type *img, int fd); > > /* Decryption key functions */ > int load_decryption_key(char *fname); > Apart the small thing above: Acked-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Stefano Babic wrote on Fri, Mar 19, 2021 at 02:59:31PM +0100: > > +#if defined(__linux__) || defined(__FreeBSD__) > > I am also interested which other options are available and if SWUpdate is > running on seome other OS.... > > IMHO you can spare this #ifdef, I do not know about further supported OSes. Ok, will resend without this ifedf and Ack added.
diff --git a/core/stream_interface.c b/core/stream_interface.c index d459010886b5..574ed47634cb 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -222,6 +222,8 @@ static int extract_files(int fd, struct swupdate_cfg *software) fdout = openfileoutput(img->extract_file); if (fdout < 0) return -1; + if (!img_check_free_space(img, fdout)) + return -1; if (copyfile(fd, &fdout, fdh.size, &offset, 0, 0, 0, &checksum, img->sha256, false, NULL, NULL) < 0) { close(fdout); return -1; diff --git a/core/util.c b/core/util.c index 0e9ad14d5020..db34d0307699 100644 --- a/core/util.c +++ b/core/util.c @@ -24,6 +24,10 @@ #include <libgen.h> #include <regex.h> +#if defined(__linux__) +#include <sys/statvfs.h> +#endif + #include "swupdate.h" #include "util.h" #include "generated/autoconf.h" @@ -951,3 +955,46 @@ long long get_output_size(struct img_type *img, bool warn) return bytes; } + +static bool check_free_space(int fd, long long size, char *fname) +{ + /* This needs OS-specific implementation because + * linux's statfs f_bsize is optimal IO size vs. statvfs f_bsize fs block size, + * and freeBSD is the opposite... + * As everything else is the same down to field names work around + * this by just defining an alias + */ +#if defined(__FreeBSD__) +#define statvfs statfs +#define fstatvfs fstatfs +#endif +#if defined(__linux__) || defined(__FreeBSD__) + struct statvfs statvfs; + + if (fstatvfs(fd, &statvfs)) { + ERROR("Statfs failed on %s, skipping free space check", fname); + return true; + } + + if (statvfs.f_bfree * statvfs.f_bsize < size) { + ERROR("Not enough free space to extract %s (needed %llu, got %lu)", + fname, size, statvfs.f_bfree * statvfs.f_bsize); + return false; + } +#endif + + return true; +} + +bool img_check_free_space(struct img_type *img, int fd) +{ + long long size; + + size = get_output_size(img, true); + + if (size <= 0) + /* Skip check if no size found */ + return true; + + return check_free_space(fd, size, img->fname); +} diff --git a/include/util.h b/include/util.h index ef487fbed21c..7b5b2a85655d 100644 --- a/include/util.h +++ b/include/util.h @@ -219,6 +219,7 @@ void free_string_array(char **nodes); int read_lines_notify(int fd, char *buf, int buf_size, int *buf_offset, LOGLEVEL level); long long get_output_size(struct img_type *img, bool warn); +bool img_check_free_space(struct img_type *img, int fd); /* Decryption key functions */ int load_decryption_key(char *fname);
copyfile would normally fail with ENOSPC on write, but if we can know beforehand that the file will not fit it is better to error early and not disrupt whatever is running. The check reuses the existing get_output_size() helper which reads the decompressed-size or decrypted-size properties depending on the file type, and is skipped altogether if no size is set (if the file is uncompressed, the size is always its size in the cpio header) No actual check that the decompressed size matches the actual size is made unless the handler requires it later, and this only checks files that are not installed directly. Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> Cc: Christian Storm <christian.storm@siemens.com> --- Changelog v1 -> v2: - use decompressed/decrypted size properties with the ubivol helper for compatibility - make freebsd use statfs instead of statvfs; looking at linux man page linux is better off with statvfs so I kept the linux code identical to v1. I downloaded a freeBSD image so I will at least compile test this early next week, didn't have time to yet -- I've only gone as far as linux tests (size not set, uncompressed file, compressed file with size set small enough to fit into partition incorrectly or big enough to be caught by statvfs) core/stream_interface.c | 2 ++ core/util.c | 47 +++++++++++++++++++++++++++++++++++++++++ include/util.h | 1 + 3 files changed, 50 insertions(+)