From b973d2d44a0bc92090ff2638a737e4a369497cb1 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 27 Jan 2022 12:06:21 -0800 Subject: [PATCH] maint: simplify memory alignment Use the new Gnulib modules alignalloc and xalignalloc to simplify some memory allocation. Also, fix some unlikely integer overflow problems. * bootstrap.conf (gnulib_modules): Add alignalloc, xalignalloc. * src/cat.c, src/copy.c, src/dd.c, src/shred.c, src/split.c: Include alignalloc.h. * src/cat.c (main): * src/copy.c (copy_reg): * src/dd.c (alloc_ibuf, alloc_obuf): * src/shred.c (dopass): * src/split.c (main): Use alignalloc/xalignalloc/alignfree instead of doing page alignment by hand. * src/cat.c (main): Check for integer overflow in page size calculations. * src/dd.c (INPUT_BLOCK_SLOP, OUTPUT_BLOCK_SLOP, MAX_BLOCKSIZE): (real_ibuf, real_obuf) [lint]: Remove; no longer needed. (cleanup) [lint]: (scanargs): Simplify. * src/ioblksize.h (io_blksize): Do not allow blocksizes largest than the largest power of two that fits in idx_t and size_t. * src/shred.c (PAGE_ALIGN_SLOP, PATTERNBUF_SIZE): Remove. --- bootstrap.conf | 2 ++ src/cat.c | 28 +++++++++++++++------------ src/copy.c | 14 +++++--------- src/dd.c | 60 ++++++++++++++++++--------------------------------------- src/ioblksize.h | 10 +++++++--- src/shred.c | 8 +++----- src/split.c | 10 +++++----- 7 files changed, 57 insertions(+), 75 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 48f355107..5ca56f917 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -26,6 +26,7 @@ avoided_gnulib_modules=' gnulib_modules=" $avoided_gnulib_modules acl + alignalloc alignof alloca announce-gen @@ -294,6 +295,7 @@ gnulib_modules=" winsz-ioctl winsz-termios write-any-file + xalignalloc xalloc xbinary-io xdectoint diff --git a/src/cat.c b/src/cat.c index 3b9ba5356..1d6f7fbff 100644 --- a/src/cat.c +++ b/src/cat.c @@ -33,6 +33,7 @@ #include #include "system.h" +#include "alignalloc.h" #include "idx.h" #include "ioblksize.h" #include "die.h" @@ -690,16 +691,17 @@ main (int argc, char **argv) || show_tabs || squeeze_blank)) { insize = MAX (insize, outsize); - inbuf = ximalloc (insize + page_size - 1); + inbuf = xalignalloc (page_size, insize); - ok &= simple_cat (ptr_align (inbuf, page_size), insize); + ok &= simple_cat (inbuf, insize); } else { - inbuf = ximalloc (insize + 1 + page_size - 1); + /* Allocate, with an extra byte for a newline sentinel. */ + inbuf = xalignalloc (page_size, insize + 1); /* Why are - (OUTSIZE - 1 + INSIZE * 4 + LINE_COUNTER_BUF_LEN + PAGE_SIZE - 1) + (OUTSIZE - 1 + INSIZE * 4 + LINE_COUNTER_BUF_LEN) bytes allocated for the output buffer? A test whether output needs to be written is done when the input @@ -717,21 +719,23 @@ main (int argc, char **argv) positions. Align the output buffer to a page size boundary, for efficiency - on some paging implementations, so add PAGE_SIZE - 1 bytes to the - request to make room for the alignment. */ + on some paging implementations. */ - char *outbuf = ximalloc (outsize - 1 + insize * 4 - + LINE_COUNTER_BUF_LEN + page_size - 1); + idx_t bufsize; + if (INT_MULTIPLY_WRAPV (insize, 4, &bufsize) + || INT_ADD_WRAPV (bufsize, outsize, &bufsize) + || INT_ADD_WRAPV (bufsize, LINE_COUNTER_BUF_LEN - 1, &bufsize)) + xalloc_die (); + char *outbuf = xalignalloc (page_size, bufsize); - ok &= cat (ptr_align (inbuf, page_size), insize, - ptr_align (outbuf, page_size), outsize, show_nonprinting, + ok &= cat (inbuf, insize, outbuf, outsize, show_nonprinting, show_tabs, number, number_nonblank, show_ends, squeeze_blank); - free (outbuf); + alignfree (outbuf); } - free (inbuf); + alignfree (inbuf); contin: if (!reading_stdin && close (input_desc) < 0) diff --git a/src/copy.c b/src/copy.c index b2e3cb1f7..4a7d9b5d9 100644 --- a/src/copy.c +++ b/src/copy.c @@ -32,6 +32,7 @@ #include "system.h" #include "acl.h" +#include "alignalloc.h" #include "backupfile.h" #include "buffer-lcm.h" #include "canonicalize.h" @@ -229,8 +230,6 @@ create_hole (int fd, char const *name, bool punch_holes, off_t size) Return true upon successful completion; print a diagnostic and return false upon error. Note that for best results, BUF should be "well"-aligned. - BUF must have sizeof(uintptr_t)-1 bytes of additional space - beyond BUF[BUF_SIZE - 1]. Set *LAST_WRITE_MADE_HOLE to true if the final operation on DEST_FD introduced a hole. Set *TOTAL_N_READ to the number of bytes read. */ @@ -1076,8 +1075,7 @@ copy_reg (char const *src_name, char const *dst_name, mode_t dst_mode, mode_t omitted_permissions, bool *new_dst, struct stat const *src_sb) { - char *buf; - char *buf_alloc = NULL; + char *buf = NULL; int dest_desc; int dest_errno; int source_desc; @@ -1292,7 +1290,6 @@ copy_reg (char const *src_name, char const *dst_name, if (data_copy_required) { /* Choose a suitable buffer size; it may be adjusted later. */ - size_t buf_alignment = getpagesize (); size_t buf_size = io_blksize (sb); size_t hole_size = ST_BLKSIZE (sb); @@ -1319,7 +1316,7 @@ copy_reg (char const *src_name, char const *dst_name, { /* Compute the least common multiple of the input and output buffer sizes, adjusting for outlandish values. */ - size_t blcm_max = MIN (SIZE_MAX, SSIZE_MAX) - buf_alignment; + size_t blcm_max = MIN (SIZE_MAX, SSIZE_MAX); size_t blcm = buffer_lcm (io_blksize (src_open_sb), buf_size, blcm_max); @@ -1337,8 +1334,7 @@ copy_reg (char const *src_name, char const *dst_name, buf_size = blcm; } - buf_alloc = xmalloc (buf_size + buf_alignment); - buf = ptr_align (buf_alloc, buf_alignment); + buf = xalignalloc (getpagesize (), buf_size); off_t n_read; bool wrote_hole_at_eof = false; @@ -1457,7 +1453,7 @@ close_src_desc: return_val = false; } - free (buf_alloc); + alignfree (buf); return return_val; } diff --git a/src/dd.c b/src/dd.c index b52ef0948..a6a3708f1 100644 --- a/src/dd.c +++ b/src/dd.c @@ -22,6 +22,7 @@ #include #include "system.h" +#include "alignalloc.h" #include "close-stream.h" #include "die.h" #include "error.h" @@ -89,20 +90,6 @@ /* Default input and output blocksize. */ #define DEFAULT_BLOCKSIZE 512 -/* How many bytes to add to the input and output block sizes before invoking - malloc. See dd_copy for details. INPUT_BLOCK_SLOP must be no less than - OUTPUT_BLOCK_SLOP, and has one more byte because of swab_buffer. */ -#define INPUT_BLOCK_SLOP page_size -#define OUTPUT_BLOCK_SLOP (page_size - 1) - -/* Maximum blocksize for the given SLOP. - Keep it smaller than MIN (IDX_MAX, SIZE_MAX) - SLOP, so that we can - allocate buffers that size. Keep it smaller than SSIZE_MAX, for - the benefit of system calls like "read". And keep it smaller than - OFF_T_MAX, for the benefit of the large-offset seek code. */ -#define MAX_BLOCKSIZE(slop) MIN (MIN (IDX_MAX, SIZE_MAX) - (slop), \ - MIN (SSIZE_MAX, OFF_T_MAX)) - /* Conversions bit masks. */ enum { @@ -239,12 +226,6 @@ static intmax_t r_truncate = 0; static char newline_character = '\n'; static char space_character = ' '; -#ifdef lint -/* Memory blocks allocated for I/O buffers and surrounding areas. */ -static char *real_ibuf; -static char *real_obuf; -#endif - /* I/O buffers. */ static char *ibuf; static char *obuf; @@ -695,9 +676,9 @@ alloc_ibuf (void) if (ibuf) return; - /* Ensure the input buffer is page aligned. */ - char *buf = malloc (input_blocksize + INPUT_BLOCK_SLOP); - if (!buf) + bool extra_byte_for_swab = !!(conversions_mask & C_SWAB); + ibuf = alignalloc (page_size, input_blocksize + extra_byte_for_swab); + if (!ibuf) { char hbuf[LONGEST_HUMAN_READABLE + 1]; die (EXIT_FAILURE, 0, @@ -706,10 +687,6 @@ alloc_ibuf (void) human_readable (input_blocksize, hbuf, human_opts | human_base_1024, 1, 1)); } -#ifdef lint - real_ibuf = buf; -#endif - ibuf = ptr_align (buf, page_size); } /* Ensure output buffer OBUF is allocated/initialized. */ @@ -722,9 +699,8 @@ alloc_obuf (void) if (conversions_mask & C_TWOBUFS) { - /* Page-align the output buffer, too. */ - char *buf = malloc (output_blocksize + OUTPUT_BLOCK_SLOP); - if (!buf) + obuf = alignalloc (page_size, output_blocksize); + if (!obuf) { char hbuf[LONGEST_HUMAN_READABLE + 1]; die (EXIT_FAILURE, 0, @@ -734,10 +710,6 @@ alloc_obuf (void) human_readable (output_blocksize, hbuf, human_opts | human_base_1024, 1, 1)); } -#ifdef lint - real_obuf = buf; -#endif - obuf = ptr_align (buf, page_size); } else { @@ -966,10 +938,9 @@ static void cleanup (void) { #ifdef lint - free (real_ibuf); - free (real_obuf); - real_ibuf = NULL; - real_obuf = NULL; + if (ibuf != obuf) + alignfree (ibuf); + alignfree (obuf); #endif if (iclose (STDIN_FILENO) != 0) @@ -1552,22 +1523,29 @@ scanargs (int argc, char *const *argv) intmax_t n_max = INTMAX_MAX; idx_t *converted_idx = NULL; + /* Maximum blocksize. Keep it smaller than IDX_MAX, so that + it fits into blocksize vars even if 1 is added for conv=swab. + Do not exceed SSIZE_MAX, for the benefit of system calls + like "read". And do not exceed OFF_T_MAX, for the + benefit of the large-offset seek code. */ + idx_t max_blocksize = MIN (IDX_MAX - 1, MIN (SSIZE_MAX, OFF_T_MAX)); + if (operand_is (name, "ibs")) { n_min = 1; - n_max = MAX_BLOCKSIZE (INPUT_BLOCK_SLOP); + n_max = max_blocksize; converted_idx = &input_blocksize; } else if (operand_is (name, "obs")) { n_min = 1; - n_max = MAX_BLOCKSIZE (OUTPUT_BLOCK_SLOP); + n_max = max_blocksize; converted_idx = &output_blocksize; } else if (operand_is (name, "bs")) { n_min = 1; - n_max = MAX_BLOCKSIZE (INPUT_BLOCK_SLOP); + n_max = max_blocksize; converted_idx = &blocksize; } else if (operand_is (name, "cbs")) diff --git a/src/ioblksize.h b/src/ioblksize.h index 8f8cd1fc2..8bd18ba05 100644 --- a/src/ioblksize.h +++ b/src/ioblksize.h @@ -16,7 +16,8 @@ /* Include this file _after_ system headers if possible. */ -/* sys/stat.h will already have been included by system.h. */ +/* sys/stat.h and minmax.h will already have been included by system.h. */ +#include "idx.h" #include "stat-size.h" @@ -71,8 +72,11 @@ and default to io_blksize() if not. */ enum { IO_BUFSIZE = 128 * 1024 }; -static inline size_t +static inline idx_t io_blksize (struct stat sb) { - return MAX (IO_BUFSIZE, ST_BLKSIZE (sb)); + /* Don’t go above the largest power of two that fits in idx_t and size_t, + as that is asking for trouble. */ + return MIN (MIN (IDX_MAX, SIZE_MAX) / 2 + 1, + MAX (IO_BUFSIZE, ST_BLKSIZE (sb))); } diff --git a/src/shred.c b/src/shred.c index 6e36b39e4..e88676380 100644 --- a/src/shred.c +++ b/src/shred.c @@ -85,6 +85,7 @@ #endif #include "system.h" +#include "alignalloc.h" #include "argmatch.h" #include "xdectoint.h" #include "die.h" @@ -412,11 +413,8 @@ dopass (int fd, struct stat const *st, char const *qname, off_t *sizep, verify (PERIODIC_OUTPUT_SIZE % 3 == 0); size_t output_size = periodic_pattern (type) ? PERIODIC_OUTPUT_SIZE : NONPERIODIC_OUTPUT_SIZE; -#define PAGE_ALIGN_SLOP (page_size - 1) /* So directio works */ #define FILLPATTERN_SIZE (((output_size + 2) / 3) * 3) /* Multiple of 3 */ -#define PATTERNBUF_SIZE (PAGE_ALIGN_SLOP + FILLPATTERN_SIZE) - void *fill_pattern_mem = xmalloc (PATTERNBUF_SIZE); - unsigned char *pbuf = ptr_align (fill_pattern_mem, page_size); + unsigned char *pbuf = xalignalloc (page_size, FILLPATTERN_SIZE); char pass_string[PASS_NAME_SIZE]; /* Name of current pass */ bool write_error = false; @@ -620,7 +618,7 @@ dopass (int fd, struct stat const *st, char const *qname, off_t *sizep, } free_pattern_mem: - free (fill_pattern_mem); + alignfree (pbuf); return other_error ? -1 : write_error; } diff --git a/src/split.c b/src/split.c index b320c2263..533c22f9f 100644 --- a/src/split.c +++ b/src/split.c @@ -29,6 +29,7 @@ #include #include "system.h" +#include "alignalloc.h" #include "die.h" #include "error.h" #include "fd-reopen.h" @@ -1300,7 +1301,7 @@ int main (int argc, char **argv) { enum Split_type split_type = type_undef; - size_t in_blk_size = 0; /* optimal block size of input file device */ + idx_t in_blk_size = 0; /* optimal block size of input file device */ size_t page_size = getpagesize (); uintmax_t k_units = 0; uintmax_t n_units = 0; @@ -1503,7 +1504,7 @@ main (int argc, char **argv) break; case IO_BLKSIZE_OPTION: - in_blk_size = xdectoumax (optarg, 1, SIZE_MAX - page_size, + in_blk_size = xdectoumax (optarg, 1, MIN (IDX_MAX, SIZE_MAX) - 1, multipliers, _("invalid IO block size"), 0); break; @@ -1585,8 +1586,7 @@ main (int argc, char **argv) if (! specified_buf_size) in_blk_size = io_blksize (in_stat_buf); - void *b = xmalloc (in_blk_size + 1 + page_size - 1); - char *buf = ptr_align (b, page_size); + char *buf = xalignalloc (page_size, in_blk_size + 1); size_t initial_read = SIZE_MAX; if (split_type == type_chunk_bytes || split_type == type_chunk_lines) @@ -1661,7 +1661,7 @@ main (int argc, char **argv) abort (); } - IF_LINT (free (b)); + IF_LINT (alignfree (buf)); if (close (STDIN_FILENO) != 0) die (EXIT_FAILURE, errno, "%s", quotef (infile)); -- 2.11.4.GIT