Using vsnprintf or its derivatives with %n can have security
vulnerability implications.

Prior to commit fef20d9c1380
("vsprintf: unify the format decoding layer for its 3 users"),
any use of %n was ignored.

Reintroduce this feature and convert the existing uses of %n
to use the return length from vsnprintf or its derivatives.

Signed-off-by: Joe Perches <j...@perches.com>
Acked-by: KOSAKI Motohiro <kosaki.motoh...@jp.fujitsu.com> (proc bits)
cc: Kees Cook <keesc...@chromium.org>
cc: Frederic Weisbecker <fweis...@gmail.com>

---

Not particularly well tested...

 fs/proc/consoles.c   |  2 +-
 fs/proc/nommu.c      | 20 ++++++-------
 fs/proc/task_mmu.c   | 18 +++++------
 fs/proc/task_nommu.c | 20 ++++++-------
 lib/vsprintf.c       | 21 ++++++-------
 net/ipv4/fib_trie.c  | 30 ++++++++-----------
 net/ipv4/ping.c      | 19 ++++++------
 net/ipv4/tcp_ipv4.c  | 84 +++++++++++++++++++++++++---------------------------
 net/ipv4/udp.c       | 19 ++++++------
 net/phonet/socket.c  | 32 ++++++++++----------
 net/sctp/objcnt.c    |  5 ++--
 11 files changed, 132 insertions(+), 138 deletions(-)

diff --git a/fs/proc/consoles.c b/fs/proc/consoles.c
index b701eaa..42f2bb7 100644
--- a/fs/proc/consoles.c
+++ b/fs/proc/consoles.c
@@ -47,7 +47,7 @@ static int show_console_dev(struct seq_file *m, void *v)
                        con_flags[a].name : ' ';
        flags[a] = 0;
 
-       seq_printf(m, "%s%d%n", con->name, con->index, &len);
+       len = seq_printf(m, "%s%d", con->name, con->index);
        len = 21 - len;
        if (len < 1)
                len = 1;
diff --git a/fs/proc/nommu.c b/fs/proc/nommu.c
index ccfd99b..91cfd19 100644
--- a/fs/proc/nommu.c
+++ b/fs/proc/nommu.c
@@ -50,16 +50,16 @@ static int nommu_region_show(struct seq_file *m, struct 
vm_region *region)
                ino = inode->i_ino;
        }
 
-       seq_printf(m,
-                  "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
-                  region->vm_start,
-                  region->vm_end,
-                  flags & VM_READ ? 'r' : '-',
-                  flags & VM_WRITE ? 'w' : '-',
-                  flags & VM_EXEC ? 'x' : '-',
-                  flags & VM_MAYSHARE ? flags & VM_SHARED ? 'S' : 's' : 'p',
-                  ((loff_t)region->vm_pgoff) << PAGE_SHIFT,
-                  MAJOR(dev), MINOR(dev), ino, &len);
+       len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
+                        region->vm_start,
+                        region->vm_end,
+                        flags & VM_READ ? 'r' : '-',
+                        flags & VM_WRITE ? 'w' : '-',
+                        flags & VM_EXEC ? 'x' : '-',
+                        flags & VM_MAYSHARE ?
+                        flags & VM_SHARED ? 'S' : 's' : 'p',
+                        ((loff_t)region->vm_pgoff) << PAGE_SHIFT,
+                        MAJOR(dev), MINOR(dev), ino);
 
        if (file) {
                len = 25 + sizeof(void *) * 6 - len;
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 107d026..f84ee9f 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -286,15 +286,15 @@ show_map_vma(struct seq_file *m, struct vm_area_struct 
*vma, int is_pid)
        if (stack_guard_page_end(vma, end))
                end -= PAGE_SIZE;
 
-       seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
-                       start,
-                       end,
-                       flags & VM_READ ? 'r' : '-',
-                       flags & VM_WRITE ? 'w' : '-',
-                       flags & VM_EXEC ? 'x' : '-',
-                       flags & VM_MAYSHARE ? 's' : 'p',
-                       pgoff,
-                       MAJOR(dev), MINOR(dev), ino, &len);
+       len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
+                        start,
+                        end,
+                        flags & VM_READ ? 'r' : '-',
+                        flags & VM_WRITE ? 'w' : '-',
+                        flags & VM_EXEC ? 'x' : '-',
+                        flags & VM_MAYSHARE ? 's' : 'p',
+                        pgoff,
+                        MAJOR(dev), MINOR(dev), ino);
 
        /*
         * Print the dentry name for named mappings, and a
diff --git a/fs/proc/task_nommu.c b/fs/proc/task_nommu.c
index 56123a6..1d7bbe5 100644
--- a/fs/proc/task_nommu.c
+++ b/fs/proc/task_nommu.c
@@ -155,16 +155,16 @@ static int nommu_vma_show(struct seq_file *m, struct 
vm_area_struct *vma,
                pgoff = (loff_t)vma->vm_pgoff << PAGE_SHIFT;
        }
 
-       seq_printf(m,
-                  "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu %n",
-                  vma->vm_start,
-                  vma->vm_end,
-                  flags & VM_READ ? 'r' : '-',
-                  flags & VM_WRITE ? 'w' : '-',
-                  flags & VM_EXEC ? 'x' : '-',
-                  flags & VM_MAYSHARE ? flags & VM_SHARED ? 'S' : 's' : 'p',
-                  pgoff,
-                  MAJOR(dev), MINOR(dev), ino, &len);
+       len = seq_printf(m, "%08lx-%08lx %c%c%c%c %08llx %02x:%02x %lu ",
+                        vma->vm_start,
+                        vma->vm_end,
+                        flags & VM_READ ? 'r' : '-',
+                        flags & VM_WRITE ? 'w' : '-',
+                        flags & VM_EXEC ? 'x' : '-',
+                        flags & VM_MAYSHARE ?
+                        flags & VM_SHARED ? 'S' : 's' : 'p',
+                        pgoff,
+                        MAJOR(dev), MINOR(dev), ino);
 
        if (file) {
                pad_len_spaces(m, len);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 26559bd..43c2ea0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1683,18 +1683,19 @@ int vsnprintf(char *buf, size_t size, const char *fmt, 
va_list args)
                        break;
 
                case FORMAT_TYPE_NRCHARS: {
+                       /* skip %n 's argument */
                        u8 qualifier = spec.qualifier;
+                       void *skip_arg;
 
-                       if (qualifier == 'l') {
-                               long *ip = va_arg(args, long *);
-                               *ip = (str - buf);
-                       } else if (_tolower(qualifier) == 'z') {
-                               size_t *ip = va_arg(args, size_t *);
-                               *ip = (str - buf);
-                       } else {
-                               int *ip = va_arg(args, int *);
-                               *ip = (str - buf);
-                       }
+                       WARN_ONCE(1, "Please remove ignored use of %%n in 
'%s'\n",
+                                 old_fmt);
+
+                       if (qualifier == 'l')
+                               skip_arg = va_arg(args, long *);
+                       else if (_tolower(qualifier) == 'z')
+                               skip_arg = va_arg(args, size_t *);
+                       else
+                               skip_arg = va_arg(args, int *);
                        break;
                }
 
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 3df6d3e..ddf1e9c 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2537,24 +2537,20 @@ static int fib_route_seq_show(struct seq_file *seq, 
void *v)
                                continue;
 
                        if (fi)
-                               seq_printf(seq,
-                                        "%s\t%08X\t%08X\t%04X\t%d\t%u\t"
-                                        "%d\t%08X\t%d\t%u\t%u%n",
-                                        fi->fib_dev ? fi->fib_dev->name : "*",
-                                        prefix,
-                                        fi->fib_nh->nh_gw, flags, 0, 0,
-                                        fi->fib_priority,
-                                        mask,
-                                        (fi->fib_advmss ?
-                                         fi->fib_advmss + 40 : 0),
-                                        fi->fib_window,
-                                        fi->fib_rtt >> 3, &len);
+                               len = seq_printf(seq, 
"%s\t%08X\t%08X\t%04X\t%d\t%u\t%d\t%08X\t%d\t%u\t%u",
+                                                (fi->fib_dev ?
+                                                 fi->fib_dev->name : "*"),
+                                                prefix,
+                                                fi->fib_nh->nh_gw, flags,
+                                                0, 0, fi->fib_priority, mask,
+                                                (fi->fib_advmss ?
+                                                 fi->fib_advmss + 40 : 0),
+                                                fi->fib_window,
+                                                fi->fib_rtt >> 3);
                        else
-                               seq_printf(seq,
-                                        "*\t%08X\t%08X\t%04X\t%d\t%u\t"
-                                        "%d\t%08X\t%d\t%u\t%u%n",
-                                        prefix, 0, flags, 0, 0, 0,
-                                        mask, 0, 0, 0, &len);
+                               len = seq_printf(seq, 
"*\t%08X\t%08X\t%04X\t%d\t%u\t%d\t%08X\t%d\t%u\t%u",
+                                                prefix, 0, flags, 0, 0, 0,
+                                                mask, 0, 0, 0);
 
                        seq_printf(seq, "%*s\n", 127 - len, "");
                }
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index d7d9882..ac8d79f 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -1081,16 +1081,15 @@ static void ping_v4_format_sock(struct sock *sp, struct 
seq_file *f,
        __u16 destp = ntohs(inet->inet_dport);
        __u16 srcp = ntohs(inet->inet_sport);
 
-       seq_printf(f, "%5d: %08X:%04X %08X:%04X"
-               " %02X %08X:%08X %02X:%08lX %08X %5u %8d %lu %d %pK %d%n",
-               bucket, src, srcp, dest, destp, sp->sk_state,
-               sk_wmem_alloc_get(sp),
-               sk_rmem_alloc_get(sp),
-               0, 0L, 0,
-               from_kuid_munged(seq_user_ns(f), sock_i_uid(sp)),
-               0, sock_i_ino(sp),
-               atomic_read(&sp->sk_refcnt), sp,
-               atomic_read(&sp->sk_drops), len);
+       *len = seq_printf(f, "%5d: %08X:%04X %08X:%04X %02X %08X:%08X 
%02X:%08lX %08X %5u %8d %lu %d %pK %d",
+                         bucket, src, srcp, dest, destp, sp->sk_state,
+                         sk_wmem_alloc_get(sp),
+                         sk_rmem_alloc_get(sp),
+                         0, 0L, 0,
+                         from_kuid_munged(seq_user_ns(f), sock_i_uid(sp)),
+                         0, sock_i_ino(sp),
+                         atomic_read(&sp->sk_refcnt), sp,
+                         atomic_read(&sp->sk_drops));
 }
 
 static int ping_v4_seq_show(struct seq_file *seq, void *v)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b14266b..1279c16 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2603,24 +2603,24 @@ static void get_openreq4(const struct sock *sk, const 
struct request_sock *req,
        const struct inet_request_sock *ireq = inet_rsk(req);
        long delta = req->expires - jiffies;
 
-       seq_printf(f, "%4d: %08X:%04X %08X:%04X"
-               " %02X %08X:%08X %02X:%08lX %08X %5u %8d %u %d %pK%n",
-               i,
-               ireq->loc_addr,
-               ntohs(inet_sk(sk)->inet_sport),
-               ireq->rmt_addr,
-               ntohs(ireq->rmt_port),
-               TCP_SYN_RECV,
-               0, 0, /* could print option size, but that is af dependent. */
-               1,    /* timers active (only the expire timer) */
-               jiffies_delta_to_clock_t(delta),
-               req->num_timeout,
-               from_kuid_munged(seq_user_ns(f), uid),
-               0,  /* non standard timer */
-               0, /* open_requests have no inode */
-               atomic_read(&sk->sk_refcnt),
-               req,
-               len);
+       *len = seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X 
%02X:%08lX %08X %5u %8d %u %d %pK",
+                         i,
+                         ireq->loc_addr,
+                         ntohs(inet_sk(sk)->inet_sport),
+                         ireq->rmt_addr,
+                         ntohs(ireq->rmt_port),
+                         TCP_SYN_RECV,
+                         0, 0, /* could print option size,
+                                * but that is af dependent.
+                                */
+                         1,    /* timers active (only the expire timer) */
+                         jiffies_delta_to_clock_t(delta),
+                         req->num_timeout,
+                         from_kuid_munged(seq_user_ns(f), uid),
+                         0,    /* non standard timer */
+                         0,    /* open_requests have no inode */
+                         atomic_read(&sk->sk_refcnt),
+                         req);
 }
 
 static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len)
@@ -2661,26 +2661,25 @@ static void get_tcp4_sock(struct sock *sk, struct 
seq_file *f, int i, int *len)
                 */
                rx_queue = max_t(int, tp->rcv_nxt - tp->copied_seq, 0);
 
-       seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X %02X:%08lX "
-                       "%08X %5u %8d %lu %d %pK %lu %lu %u %u %d%n",
-               i, src, srcp, dest, destp, sk->sk_state,
-               tp->write_seq - tp->snd_una,
-               rx_queue,
-               timer_active,
-               jiffies_delta_to_clock_t(timer_expires - jiffies),
-               icsk->icsk_retransmits,
-               from_kuid_munged(seq_user_ns(f), sock_i_uid(sk)),
-               icsk->icsk_probes_out,
-               sock_i_ino(sk),
-               atomic_read(&sk->sk_refcnt), sk,
-               jiffies_to_clock_t(icsk->icsk_rto),
-               jiffies_to_clock_t(icsk->icsk_ack.ato),
-               (icsk->icsk_ack.quick << 1) | icsk->icsk_ack.pingpong,
-               tp->snd_cwnd,
-               sk->sk_state == TCP_LISTEN ?
-                   (fastopenq ? fastopenq->max_qlen : 0) :
-                   (tcp_in_initial_slowstart(tp) ? -1 : tp->snd_ssthresh),
-               len);
+       *len = seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X 
%02X:%08lX %08X %5u %8d %lu %d %pK %lu %lu %u %u %d",
+                         i, src, srcp, dest, destp, sk->sk_state,
+                         tp->write_seq - tp->snd_una,
+                         rx_queue,
+                         timer_active,
+                         jiffies_delta_to_clock_t(timer_expires - jiffies),
+                         icsk->icsk_retransmits,
+                         from_kuid_munged(seq_user_ns(f), sock_i_uid(sk)),
+                         icsk->icsk_probes_out,
+                         sock_i_ino(sk),
+                         atomic_read(&sk->sk_refcnt), sk,
+                         jiffies_to_clock_t(icsk->icsk_rto),
+                         jiffies_to_clock_t(icsk->icsk_ack.ato),
+                         (icsk->icsk_ack.quick << 1) | icsk->icsk_ack.pingpong,
+                         tp->snd_cwnd,
+                         sk->sk_state == TCP_LISTEN ?
+                         (fastopenq ? fastopenq->max_qlen : 0) :
+                         (tcp_in_initial_slowstart(tp) ? -1 :
+                          tp->snd_ssthresh));
 }
 
 static void get_timewait4_sock(const struct inet_timewait_sock *tw,
@@ -2695,11 +2694,10 @@ static void get_timewait4_sock(const struct 
inet_timewait_sock *tw,
        destp = ntohs(tw->tw_dport);
        srcp  = ntohs(tw->tw_sport);
 
-       seq_printf(f, "%4d: %08X:%04X %08X:%04X"
-               " %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %pK%n",
-               i, src, srcp, dest, destp, tw->tw_substate, 0, 0,
-               3, jiffies_delta_to_clock_t(delta), 0, 0, 0, 0,
-               atomic_read(&tw->tw_refcnt), tw, len);
+       *len = seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X 
%02X:%08lX %08X %5d %8d %d %d %pK",
+                         i, src, srcp, dest, destp, tw->tw_substate, 0, 0,
+                         3, jiffies_delta_to_clock_t(delta), 0, 0, 0, 0,
+                         atomic_read(&tw->tw_refcnt), tw);
 }
 
 #define TMPSZ 150
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 74d2c95..ddc24a2d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2158,16 +2158,15 @@ static void udp4_format_sock(struct sock *sp, struct 
seq_file *f,
        __u16 destp       = ntohs(inet->inet_dport);
        __u16 srcp        = ntohs(inet->inet_sport);
 
-       seq_printf(f, "%5d: %08X:%04X %08X:%04X"
-               " %02X %08X:%08X %02X:%08lX %08X %5u %8d %lu %d %pK %d%n",
-               bucket, src, srcp, dest, destp, sp->sk_state,
-               sk_wmem_alloc_get(sp),
-               sk_rmem_alloc_get(sp),
-               0, 0L, 0,
-               from_kuid_munged(seq_user_ns(f), sock_i_uid(sp)),
-               0, sock_i_ino(sp),
-               atomic_read(&sp->sk_refcnt), sp,
-               atomic_read(&sp->sk_drops), len);
+       *len = seq_printf(f, "%5d: %08X:%04X %08X:%04X %02X %08X:%08X 
%02X:%08lX %08X %5u %8d %lu %d %pK %d",
+                         bucket, src, srcp, dest, destp, sp->sk_state,
+                         sk_wmem_alloc_get(sp),
+                         sk_rmem_alloc_get(sp),
+                         0, 0L, 0,
+                         from_kuid_munged(seq_user_ns(f), sock_i_uid(sp)),
+                         0, sock_i_ino(sp),
+                         atomic_read(&sp->sk_refcnt), sp,
+                         atomic_read(&sp->sk_drops));
 }
 
 int udp4_seq_show(struct seq_file *seq, void *v)
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 77e38f7..553f896 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -598,21 +598,20 @@ static int pn_sock_seq_show(struct seq_file *seq, void *v)
        int len;
 
        if (v == SEQ_START_TOKEN)
-               seq_printf(seq, "%s%n", "pt  loc  rem rs st tx_queue rx_queue "
-                       "  uid inode ref pointer drops", &len);
+               len = seq_puts(seq, "pt  loc  rem rs st tx_queue rx_queue   uid 
inode ref pointer drops");
        else {
                struct sock *sk = v;
                struct pn_sock *pn = pn_sk(sk);
 
-               seq_printf(seq, "%2d %04X:%04X:%02X %02X %08X:%08X %5d %lu "
-                       "%d %pK %d%n",
-                       sk->sk_protocol, pn->sobject, pn->dobject,
-                       pn->resource, sk->sk_state,
-                       sk_wmem_alloc_get(sk), sk_rmem_alloc_get(sk),
-                       from_kuid_munged(seq_user_ns(seq), sock_i_uid(sk)),
-                       sock_i_ino(sk),
-                       atomic_read(&sk->sk_refcnt), sk,
-                       atomic_read(&sk->sk_drops), &len);
+               len = seq_printf(seq, "%2d %04X:%04X:%02X %02X %08X:%08X %5d 
%lu %d %pK %d",
+                                sk->sk_protocol, pn->sobject, pn->dobject,
+                                pn->resource, sk->sk_state,
+                                sk_wmem_alloc_get(sk), sk_rmem_alloc_get(sk),
+                                from_kuid_munged(seq_user_ns(seq),
+                                                 sock_i_uid(sk)),
+                                sock_i_ino(sk),
+                                atomic_read(&sk->sk_refcnt), sk,
+                                atomic_read(&sk->sk_drops));
        }
        seq_printf(seq, "%*s\n", 127 - len, "");
        return 0;
@@ -788,15 +787,16 @@ static int pn_res_seq_show(struct seq_file *seq, void *v)
        int len;
 
        if (v == SEQ_START_TOKEN)
-               seq_printf(seq, "%s%n", "rs   uid inode", &len);
+               len = seq_puts(seq, "rs   uid inode");
        else {
                struct sock **psk = v;
                struct sock *sk = *psk;
 
-               seq_printf(seq, "%02X %5u %lu%n",
-                          (int) (psk - pnres.sk),
-                          from_kuid_munged(seq_user_ns(seq), sock_i_uid(sk)),
-                          sock_i_ino(sk), &len);
+               len = seq_printf(seq, "%02X %5u %lu",
+                                (int) (psk - pnres.sk),
+                                from_kuid_munged(seq_user_ns(seq),
+                                                 sock_i_uid(sk)),
+                                sock_i_ino(sk));
        }
        seq_printf(seq, "%*s\n", 63 - len, "");
        return 0;
diff --git a/net/sctp/objcnt.c b/net/sctp/objcnt.c
index 5ea573b..8034325 100644
--- a/net/sctp/objcnt.c
+++ b/net/sctp/objcnt.c
@@ -82,8 +82,9 @@ static int sctp_objcnt_seq_show(struct seq_file *seq, void *v)
        int i, len;
 
        i = (int)*(loff_t *)v;
-       seq_printf(seq, "%s: %d%n", sctp_dbg_objcnt[i].label,
-                               atomic_read(sctp_dbg_objcnt[i].counter), &len);
+       len = seq_printf(seq, "%s: %d",
+                        sctp_dbg_objcnt[i].label,
+                        atomic_read(sctp_dbg_objcnt[i].counter));
        seq_printf(seq, "%*s\n", 127 - len, "");
        return 0;
 }
-- 
1.8.1.2.459.gbcd45b4.dirty



_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to