diff mbox series

atm: iphase: Avoid copying pointers to user space.

Message ID 20190514151205.5143-1-huangfq.daxian@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series atm: iphase: Avoid copying pointers to user space. | expand

Commit Message

Fuqian Huang May 14, 2019, 3:11 p.m. UTC
When ia_cmds.sub_cmd is MEMDUMP_DEV in ia_ioctl,
nullify the pointer fields of iadev before copying
the whole structure to user space.

Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
 drivers/atm/iphase.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 70 insertions(+), 3 deletions(-)

Comments

David Miller May 14, 2019, 11:01 p.m. UTC | #1
From: Fuqian Huang <huangfq.daxian@gmail.com>
Date: Tue, 14 May 2019 23:11:59 +0800

> When ia_cmds.sub_cmd is MEMDUMP_DEV in ia_ioctl,
> nullify the pointer fields of iadev before copying
> the whole structure to user space.
> 
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>

Honestly I'd rather you just remove the MEMDUMP_DEV ioctl altogether,
there is now ay that information is useful in any way whatsoever.

Thank you.
diff mbox series

Patch

diff --git a/drivers/atm/iphase.c b/drivers/atm/iphase.c
index 5278c57dce73..3ca73625fb1a 100644
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -2746,11 +2746,71 @@  static int ia_change_qos(struct atm_vcc *vcc, struct atm_qos *qos, int flags)
 	IF_EVENT(printk(">ia_change_qos\n");)  
 	return 0;  
 }  
+
+static void ia_nullify_pointers(IADEV *src, IADEV *dest)
+{
+	memset(dest, 0, sizeof(IADEV));
+	dest->tx_dma_q.qlen = src->tx_dma_q.qlen;
+	dest->tx_dma_q.lock = src->tx_dma_q.lock;
+	dest->tx_backlog.qlen = src->tx_backlog.qlen;
+	dest->tx_backlog.lock = src->tx_backlog.lock;
+	dest->tx_lock = src->tx_lock;
+	dest->tx_return_q.data.timestamp = src->tx_return_q.data.timestamp;
+	dest->close_pending = src->close_pending;
+	dest->close_wait.lock = src->close_wait.lock;
+	dest->timeout_wait.lock = src->timeout_wait.lock;
+	dest->num_tx_desc = src->num_tx_desc;
+	dest->tx_buf_sz = src->tx_buf_sz;
+	dest->rate_limit = src->rate_limit;
+	dest->tx_cell_cnt = src->tx_cell_cnt;
+	dest->tx_pkt_cnt = src->tx_pkt_cnt;
+	dest->rx_dma_q.qlen = src->rx_dma_q.qlen;
+	dest->rx_dma_q.lock = src->rx_dma_q.lock;
+	dest->rx_lock = src->rx_lock;
+	dest->num_rx_desc = src->num_rx_desc;
+	dest->rx_buf_sz = src->rx_buf_sz;
+	dest->rxing = src->rxing;
+	dest->rx_pkt_ram = src->rx_pkt_ram;
+	dest->rx_tmp_cnt = src->rx_tmp_cnt;
+	dest->rx_tmp_jif = src->rx_tmp_jif;
+	dest->drop_rxpkt = src->drop_rxpkt;
+	dest->drop_rxcell = src->drop_rxcell;
+	dest->rx_cell_cnt = src->rx_cell_cnt;
+	dest->rx_pkt_cnt = src->rx_pkt_cnt;
+	dest->mem = src->mem;
+	dest->real_base = src->real_base;
+	dest->pci_map_size = src->pci_map_size;
+	dest->irq = src->irq;
+	dest->bus = src->bus;
+	dest->dev_fn = src->dev_fn;
+	dest->phy_type = src->phy_type;
+	dest->num_vc = src->num_vc;
+	dest->memSize = src->memSize;
+	dest->memType = src->memType;
+	dest->ffL = src->ffL;
+	dest->rfL = src->rfL;
+	dest->carrier_detect = src->carrier_detect;
+	dest->tx_dma_cnt = src->tx_dma_cnt;
+	dest->rx_dma_cnt = src->rx_dma_cnt;
+	dest->NumEnabledCBR = src->NumEnabledCBR;
+	dest->rx_mark_cnt = src->rx_mark_cnt;
+	dest->CbrTotEntries = src->CbrTotEntries;
+	dest->CbrRemEntries = src->CbrRemEntries;
+	dest->CbrEntryPt = src->CbrEntryPt;
+	dest->Granularity = src->Granularity;
+	dest->sum_mcr = src->sum_mcr;
+	dest->sum_cbr = src->sum_cbr;
+	dest->LineRate = src->LineRate;
+	dest->n_abr = src->n_abr;
+	dest->host_tcq_wr = src->host_tcq_wr;
+	dest->tx_dle_dma = src->tx_dle_dma;
+	dest->rx_dle_dma = src->rx_dle_dma;
+}
   
 static int ia_ioctl(struct atm_dev *dev, unsigned int cmd, void __user *arg)  
 {  
    IA_CMDBUF ia_cmds;
-   IADEV *iadev;
+   IADEV *iadev, *output;
    int i, board;
    u16 __user *tmps;
    IF_EVENT(printk(">ia_ioctl\n");)  
@@ -2769,8 +2829,15 @@  static int ia_ioctl(struct atm_dev *dev, unsigned int cmd, void __user *arg)
 	switch (ia_cmds.sub_cmd) {
        	  case MEMDUMP_DEV:     
 	     if (!capable(CAP_NET_ADMIN)) return -EPERM;
-	     if (copy_to_user(ia_cmds.buf, iadev, sizeof(IADEV)))
-                return -EFAULT;
+	     output = kmalloc(sizeof(IADEV), GFP_KERNEL);
+	     if (!output)
+		     return -ENOMEM;
+	     ia_nullify_pointers(iadev, output);
+	     if (copy_to_user(ia_cmds.buf, output, sizeof(IADEV))) {
+		     kfree(output);
+		     return -EFAULT;
+	     }
+	     kfree(output);
              ia_cmds.status = 0;
              break;
           case MEMDUMP_SEGREG: