From 768c1d65836d5f91c4ac51fbc73ff69ce48a1048 Mon Sep 17 00:00:00 2001 From: Shaun Tancheff Date: Tue, 2 Jul 2024 14:07:46 +0700 Subject: [PATCH] LU-17000 utils: interger overflow fixes CoverityID: 426402 ("Logical vs. bitwise operator") LIBCFS_ALLOC_PRE() Add extra parenthesis to clarify && vs & precedence CoverityID: 429655 ("Overflowed integer argument") LIBCFS_FREE() Use size_t to avoid integer overflow CoverityID: 429629 ("Overflowed integer argument") jobid_interpret_string() Prevent joblen from becoming negative, truncate if necessary. CoverityID: 429557 ("Overflowed constant") ll_stats_pid_write() if len is 0 prevent stack corruption via kernbuf CoverityID: 429646 ("Overflowed integer argument") llog_pack_buffer() ssize_t read(): prevent int overflow if read() returns > INT_MAX CoverityID: 429630 ("Overflowed integer argument") readline() in cacheio.c ssize_t read(): prevent int overflow if read() returns > INT_MAX CoverityID: 429624 ("Overflowed integer argument") osd_read() passes loff_t size to osd_ldiskfs_readlink, update osd_ldiskfs_readlink to accept size_t length to avoid a theoretical overflow Signed-off-by: Shaun Tancheff Change-Id: Ica8a5e1ce58e540016e4bc101763f835eed2c2f7 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/55588 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Alexander Boyko Reviewed-by: Arshad Hussain Reviewed-by: Petros Koutoupis Reviewed-by: Oleg Drokin --- libcfs/include/libcfs/libcfs_private.h | 6 +++--- lustre/llite/lproc_llite.c | 2 ++ lustre/obdclass/jobid.c | 3 +++ lustre/osd-ldiskfs/osd_io.c | 3 ++- lustre/utils/gss/cacheio.c | 12 ++++++------ lustre/utils/gss/cacheio.h | 2 +- lustre/utils/gss/svcgssd_proc.c | 2 +- lustre/utils/llog_reader.c | 14 ++++++++------ 8 files changed, 26 insertions(+), 18 deletions(-) diff --git a/libcfs/include/libcfs/libcfs_private.h b/libcfs/include/libcfs/libcfs_private.h index 89690ef..e1110d7 100644 --- a/libcfs/include/libcfs/libcfs_private.h +++ b/libcfs/include/libcfs/libcfs_private.h @@ -157,7 +157,7 @@ do { \ #define LIBCFS_ALLOC_PRE(size, mask) \ do { \ LASSERT(!in_interrupt() || \ - ((size) <= LIBCFS_VMALLOC_SIZE && \ + (((size) <= LIBCFS_VMALLOC_SIZE) && \ ((mask) & GFP_ATOMIC)) != 0); \ } while (0) @@ -227,9 +227,9 @@ do { \ #define LIBCFS_FREE(ptr, size) \ do { \ - int s = (size); \ + size_t s = (size); \ if (unlikely((ptr) == NULL)) { \ - CERROR("LIBCFS: free NULL '" #ptr "' (%d bytes) at " \ + CERROR("LIBCFS: free NULL '" #ptr "' (%zd bytes) at " \ "%s:%d\n", s, __FILE__, __LINE__); \ break; \ } \ diff --git a/lustre/llite/lproc_llite.c b/lustre/llite/lproc_llite.c index 586a9c8..2e41ae4 100644 --- a/lustre/llite/lproc_llite.c +++ b/lustre/llite/lproc_llite.c @@ -114,6 +114,8 @@ static s64 ll_stats_pid_write(const char __user *buf, size_t len) char kernbuf[16]; int rc; + if (len == 0) + return -EINVAL; rc = kstrtoull_from_user(buf, len, 0, &value); if (rc < 0 && len < sizeof(kernbuf)) { if (copy_from_user(kernbuf, buf, len)) diff --git a/lustre/obdclass/jobid.c b/lustre/obdclass/jobid.c index 3315bd4..87b167f 100644 --- a/lustre/obdclass/jobid.c +++ b/lustre/obdclass/jobid.c @@ -768,6 +768,9 @@ static int jobid_interpret_string(const char *jobfmt, char *jobid, l = 0; break; } + /* truncate jobid if it is too long */ + if (l > joblen) + l = joblen; jobid += l; joblen -= l; } diff --git a/lustre/osd-ldiskfs/osd_io.c b/lustre/osd-ldiskfs/osd_io.c index 103ed29..3efe9d9 100644 --- a/lustre/osd-ldiskfs/osd_io.c +++ b/lustre/osd-ldiskfs/osd_io.c @@ -1614,7 +1614,8 @@ static int osd_read_prep(const struct lu_env *env, struct dt_object *dt, * * which doesn't work for globally shared files like /last_rcvd. */ -static int osd_ldiskfs_readlink(struct inode *inode, char *buffer, int buflen) +static int osd_ldiskfs_readlink(struct inode *inode, char *buffer, + size_t buflen) { struct ldiskfs_inode_info *ei = LDISKFS_I(inode); diff --git a/lustre/utils/gss/cacheio.c b/lustre/utils/gss/cacheio.c index d3247cd..12403e8 100644 --- a/lustre/utils/gss/cacheio.c +++ b/lustre/utils/gss/cacheio.c @@ -266,14 +266,14 @@ int qword_get_int(char **bpp, int *anint) #define READLINE_BUFFER_INCREMENT 2048 -int readline(int fd, char **buf, int *lenp) +int readline(int fd, char **buf, ssize_t *lenp) { /* read a line into *buf, which is malloced *len long * realloc if needed until we find a \n * nul out the \n and return * 0 of eof, 1 of success */ - int len; + ssize_t len; if (*lenp == 0) { char *b = malloc(READLINE_BUFFER_INCREMENT); @@ -284,7 +284,7 @@ int readline(int fd, char **buf, int *lenp) } len = read(fd, *buf, *lenp); if (len <= 0) { - printerr(0, "readline: read error: len %d errno %d (%s)\n", + printerr(0, "readline: read error: len %zd errno %d (%s)\n", len, errno, strerror(errno)); return 0; } @@ -293,7 +293,7 @@ int readline(int fd, char **buf, int *lenp) * so we have to keep reading after re-alloc */ char *new; - int nl; + ssize_t nl; *lenp += READLINE_BUFFER_INCREMENT; new = realloc(*buf, *lenp); if (new == NULL) @@ -301,14 +301,14 @@ int readline(int fd, char **buf, int *lenp) *buf = new; nl = read(fd, *buf +len, *lenp - len); if (nl <= 0 ) { - printerr(0, "readline: read error: len %d " + printerr(0, "readline: read error: len %zd " "errno %d (%s)\n", nl, errno, strerror(errno)); return 0; } len += nl; } (*buf)[len-1] = 0; - printerr(3, "readline: read %d chars into buffer of size %d:\n%s\n", + printerr(3, "readline: read %zd chars into buffer of size %zd:\n%s\n", len, *lenp, *buf); return 1; } diff --git a/lustre/utils/gss/cacheio.h b/lustre/utils/gss/cacheio.h index 365fcf1..104f8bc 100644 --- a/lustre/utils/gss/cacheio.h +++ b/lustre/utils/gss/cacheio.h @@ -42,7 +42,7 @@ int qword_print(FILE *f, const char *str); int qword_printhex(FILE *f, char *str, int slen); void qword_printint(FILE *f, int num); int qword_eol(FILE *f); -int readline(int fd, char **buf, int *lenp); +int readline(int fd, char **buf, ssize_t *lenp); int qword_get(char **bpp, char *dest, int bufsize); int qword_get_int(char **bpp, int *anint); diff --git a/lustre/utils/gss/svcgssd_proc.c b/lustre/utils/gss/svcgssd_proc.c index 8cb61fc..3aa2d19 100644 --- a/lustre/utils/gss/svcgssd_proc.c +++ b/lustre/utils/gss/svcgssd_proc.c @@ -907,7 +907,7 @@ int handle_channel_request(int fd) char out_handle_buf[15]; uint32_t lustre_mech; static char *lbuf; - static int lbuflen; + static ssize_t lbuflen; static char *cp; int get_len; int rc; diff --git a/lustre/utils/llog_reader.c b/lustre/utils/llog_reader.c index 05c8fcf..c7d6da2 100644 --- a/lustre/utils/llog_reader.c +++ b/lustre/utils/llog_reader.c @@ -223,7 +223,9 @@ int llog_pack_buffer(int fd, struct llog_log_hdr **llog, struct llog_rec_hdr ***recs, int *recs_number) { - int rc = 0, recs_num, rd = 0; + ssize_t rd = 0; + ssize_t nbytes; + int rc = 0, recs_num; long long file_size; struct stat st; char *file_buf = NULL, *recs_buf = NULL; @@ -260,15 +262,15 @@ int llog_pack_buffer(int fd, struct llog_log_hdr **llog, *llog = (struct llog_log_hdr *)file_buf; do { - rc = read(fd, file_buf + rd, file_size - rd); - if (rc > 0) - rd += rc; - } while (rc > 0 && rd < file_size); + nbytes = read(fd, file_buf + rd, file_size - rd); + if (nbytes > 0) + rd += nbytes; + } while (nbytes > 0 && rd < file_size); if (rd < file_size) { rc = rc < 0 ? -errno : -EIO; llapi_error(LLAPI_MSG_ERROR, rc, - "Error reading llog header: need %zd, got %d", + "Error reading llog header: need %zd, got %zd", sizeof(**llog), rd); goto clear_file_buf; } -- 1.8.3.1