| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Jul 2013 12:28:07 +0800
From: Chen Gang <gang.chen@...anux.com>
To: reiserfs-devel@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Al Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] reiserfs: check/extend buffer length for printing functions
I have given a simple test for it.
for current REISERFS_MAX_ERROR_BUF (error_buffer[4096]), it will report
the full message warnings.
[root@...p122 ~]# mount /dev/sda11 /mnt/sda11
[root@...p122 ~]# dmesg | grep reiser
[ 423.421532] REISERFS warning (device sda11): reiserfs_fill_super: CONFIG_REISERFS_CHECK is set ON
[ 423.421537] REISERFS warning (device sda11): reiserfs_fill_super: - it is slow mode for debugging.
decreasing REISERFS_MAX_ERROR_BUF to 11 (error_buffer[11]), it will
report the truncated message warnings (the tail 10 chars)
[root@...p122 ~]# mount /dev/sda11 /mnt/sda11
[root@...p122 ~]# dmesg | grep reiser
[ 44.236875] REISERFS warning (device sda11): reiserfs_fill_super: CONFIG_REI
[ 44.236882] REISERFS warning (device sda11): reiserfs_fill_super: - it is sl
If request the additional test, please let me know, I should perform
(better to provide the related test plan)
Thanks.
On 07/17/2013 04:48 PM, Chen Gang wrote:
> If format string and/or error string are larger than 1024, it will
> cause memory overflow.
>
> So need check the format string buffer length before process it.
>
> Also need use (v)snprintf() instread of (v)sprintf() for error buffer
> to be sure of maximize length limitation.
>
> Normally, the error buffer length need be much larger than format
> buffer length, so extend the error buffer length to 4096.
>
> When adding new code, also need let them within 80 column.
>
>
> Signed-off-by: Chen Gang <gang.chen@...anux.com>
> ---
> fs/reiserfs/prints.c | 90 ++++++++++++++++++++++++++++++-------------------
> 1 files changed, 55 insertions(+), 35 deletions(-)
>
> diff --git a/fs/reiserfs/prints.c b/fs/reiserfs/prints.c
> index c0b1112..6b7581e 100644
> --- a/fs/reiserfs/prints.c
> +++ b/fs/reiserfs/prints.c
> @@ -10,8 +10,13 @@
>
> #include <stdarg.h>
>
> -static char error_buf[1024];
> -static char fmt_buf[1024];
> +#define REISERFS_MAX_FMT_BUF 1024
> +#define REISERFS_MAX_ERROR_BUF 4096
> +#define REISERFS_ERR_BUF_LEFT(pos, base) \
> + (REISERFS_MAX_ERROR_BUF - ((pos) - (base)))
> +
> +static char error_buf[REISERFS_MAX_FMT_BUF];
> +static char fmt_buf[REISERFS_MAX_ERROR_BUF];
> static char off_buf[80];
>
> static char *reiserfs_cpu_offset(struct cpu_key *key)
> @@ -76,72 +81,74 @@ static char *le_type(struct reiserfs_key *key)
> }
>
> /* %k */
> -static void sprintf_le_key(char *buf, struct reiserfs_key *key)
> +static void sprintf_le_key(char *buf, int left, struct reiserfs_key *key)
> {
> if (key)
> - sprintf(buf, "[%d %d %s %s]", le32_to_cpu(key->k_dir_id),
> + snprintf(buf, left, "[%d %d %s %s]", le32_to_cpu(key->k_dir_id),
> le32_to_cpu(key->k_objectid), le_offset(key),
> le_type(key));
> else
> - sprintf(buf, "[NULL]");
> + snprintf(buf, left, "[NULL]");
> }
>
> /* %K */
> -static void sprintf_cpu_key(char *buf, struct cpu_key *key)
> +static void sprintf_cpu_key(char *buf, int left, struct cpu_key *key)
> {
> if (key)
> - sprintf(buf, "[%d %d %s %s]", key->on_disk_key.k_dir_id,
> + snprintf(buf, left, "[%d %d %s %s]", key->on_disk_key.k_dir_id,
> key->on_disk_key.k_objectid, reiserfs_cpu_offset(key),
> cpu_type(key));
> else
> - sprintf(buf, "[NULL]");
> + snprintf(buf, left, "[NULL]");
> }
>
> -static void sprintf_de_head(char *buf, struct reiserfs_de_head *deh)
> +static void sprintf_de_head(char *buf, int left, struct reiserfs_de_head *deh)
> {
> if (deh)
> - sprintf(buf,
> + snprintf(buf, left,
> "[offset=%d dir_id=%d objectid=%d location=%d state=%04x]",
> deh_offset(deh), deh_dir_id(deh), deh_objectid(deh),
> deh_location(deh), deh_state(deh));
> else
> - sprintf(buf, "[NULL]");
> + snprintf(buf, left, "[NULL]");
>
> }
>
> -static void sprintf_item_head(char *buf, struct item_head *ih)
> +static void sprintf_item_head(char *buf, int left, struct item_head *ih)
> {
> if (ih) {
> - strcpy(buf,
> + snprintf(buf, left, "%s",
> (ih_version(ih) == KEY_FORMAT_3_6) ? "*3.6* " : "*3.5*");
> - sprintf_le_key(buf + strlen(buf), &(ih->ih_key));
> - sprintf(buf + strlen(buf), ", item_len %d, item_location %d, "
> - "free_space(entry_count) %d",
> + sprintf_le_key(buf + strlen(buf), left - strlen(buf),
> + &(ih->ih_key));
> + snprintf(buf + strlen(buf), left - strlen(buf),
> + ", item_len %d, item_location %d, free_space(entry_count) %d",
> ih_item_len(ih), ih_location(ih), ih_free_space(ih));
> } else
> - sprintf(buf, "[NULL]");
> + snprintf(buf, left, "[NULL]");
> }
>
> -static void sprintf_direntry(char *buf, struct reiserfs_dir_entry *de)
> +static void sprintf_direntry(char *buf, int left, struct reiserfs_dir_entry *de)
> {
> char name[20];
>
> memcpy(name, de->de_name, de->de_namelen > 19 ? 19 : de->de_namelen);
> name[de->de_namelen > 19 ? 19 : de->de_namelen] = 0;
> - sprintf(buf, "\"%s\"==>[%d %d]", name, de->de_dir_id, de->de_objectid);
> + snprintf(buf, left, "\"%s\"==>[%d %d]",
> + name, de->de_dir_id, de->de_objectid);
> }
>
> -static void sprintf_block_head(char *buf, struct buffer_head *bh)
> +static void sprintf_block_head(char *buf, int left, struct buffer_head *bh)
> {
> - sprintf(buf, "level=%d, nr_items=%d, free_space=%d rdkey ",
> + snprintf(buf, left, "level=%d, nr_items=%d, free_space=%d rdkey ",
> B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh));
> }
>
> -static void sprintf_buffer_head(char *buf, struct buffer_head *bh)
> +static void sprintf_buffer_head(char *buf, int left, struct buffer_head *bh)
> {
> char b[BDEVNAME_SIZE];
>
> - sprintf(buf,
> + snprintf(buf, left,
> "dev %s, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)",
> bdevname(bh->b_bdev, b), bh->b_size,
> (unsigned long long)bh->b_blocknr, atomic_read(&(bh->b_count)),
> @@ -151,9 +158,9 @@ static void sprintf_buffer_head(char *buf, struct buffer_head *bh)
> buffer_locked(bh) ? "LOCKED" : "UNLOCKED");
> }
>
> -static void sprintf_disk_child(char *buf, struct disk_child *dc)
> +static void sprintf_disk_child(char *buf, int left, struct disk_child *dc)
> {
> - sprintf(buf, "[dc_number=%d, dc_size=%u]", dc_block_number(dc),
> + snprintf(buf, left, "[dc_number=%d, dc_size=%u]", dc_block_number(dc),
> dc_size(dc));
> }
>
> @@ -190,8 +197,16 @@ static void prepare_error_buf(const char *fmt, va_list args)
> char *fmt1 = fmt_buf;
> char *k;
> char *p = error_buf;
> + int left = REISERFS_ERR_BUF_LEFT(p, error_buf);
> int what;
>
> + if (strlen(fmt) >= REISERFS_MAX_FMT_BUF) {
> + printk(KERN_CRIT
> + "REISERFS error (format buffer too long, more than %d): %s\n",
> + REISERFS_MAX_FMT_BUF, fmt);
> + return;
> + }
> +
> spin_lock(&error_lock);
>
> strcpy(fmt1, fmt);
> @@ -199,46 +214,51 @@ static void prepare_error_buf(const char *fmt, va_list args)
> while ((k = is_there_reiserfs_struct(fmt1, &what)) != NULL) {
> *k = 0;
>
> - p += vsprintf(p, fmt1, args);
> + p += vsnprintf(p, left, fmt1, args);
> + left = REISERFS_ERR_BUF_LEFT(p, error_buf);
>
> switch (what) {
> case 'k':
> - sprintf_le_key(p, va_arg(args, struct reiserfs_key *));
> + sprintf_le_key(p, left,
> + va_arg(args, struct reiserfs_key *));
> break;
> case 'K':
> - sprintf_cpu_key(p, va_arg(args, struct cpu_key *));
> + sprintf_cpu_key(p, left,
> + va_arg(args, struct cpu_key *));
> break;
> case 'h':
> - sprintf_item_head(p, va_arg(args, struct item_head *));
> + sprintf_item_head(p, left,
> + va_arg(args, struct item_head *));
> break;
> case 't':
> - sprintf_direntry(p,
> + sprintf_direntry(p, left,
> va_arg(args,
> struct reiserfs_dir_entry *));
> break;
> case 'y':
> - sprintf_disk_child(p,
> + sprintf_disk_child(p, left,
> va_arg(args, struct disk_child *));
> break;
> case 'z':
> - sprintf_block_head(p,
> + sprintf_block_head(p, left,
> va_arg(args, struct buffer_head *));
> break;
> case 'b':
> - sprintf_buffer_head(p,
> + sprintf_buffer_head(p, left,
> va_arg(args, struct buffer_head *));
> break;
> case 'a':
> - sprintf_de_head(p,
> + sprintf_de_head(p, left,
> va_arg(args,
> struct reiserfs_de_head *));
> break;
> }
>
> p += strlen(p);
> + left = REISERFS_ERR_BUF_LEFT(p, error_buf);
> fmt1 = k + 2;
> }
> - vsprintf(p, fmt1, args);
> + vsnprintf(p, left, fmt1, args);
> spin_unlock(&error_lock);
>
> }
>
--
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists