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@dhcp122 ~]# mount /dev/sda11 /mnt/sda11
[root@dhcp122 ~]# 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@dhcp122 ~]# mount /dev/sda11 /mnt/sda11
[root@dhcp122 ~]# 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.c...@asianux.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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to