[rds-devel] Linux RDS socket : OOB Write in rds_message_alloc_sgs()

[Santosh Shilimkar]

The fix is in mainline and marked for stable but am not sure
if it will get back-ported all the way.

https://patchwork.kernel.org/patch/10128783/

On 1/2/2018 8:23 AM, Mohamed Ghannam wrote:
> Hi,
> 
> I reported this bug a while ago and still have no answer, so this mail 
> is just a reminder and making sure that it's been well received,
> 
> Cheers,
> Mohamed
> 
> 2017-12-17 12:44 GMT+00:00 Mohamed Ghannam <simo.ghannam at gmail.com 
> <mailto:simo.ghannam at gmail.com>>:
> 
>     Hi,
> 
>     Here is a bug I found in RDS socket, it's an Out of bound write
>     The vulnerability affects Linux Kernel from v2.6.29-rc6 to the
>     latest version, it's been introduced in this commit
>     eff5f53bef75c0864a5da06bb688939092b848dc
> 
>     The vulnerability occurs when we deal with ancullary data (through
>     cmsghdr)
> 
>     int rds_sendmsg(struct socket *sock, struct msghdr *msg, size_t
>     payload_len)
>     {
>     ...
> 
>     /* size of rm including all sgs */
>     ret = rds_rm_size(msg, payload_len);<--- (1)
>     if (ret < 0)
>     goto out;
>     ...
>     ...
>     ret = rds_cmsg_send(rs, rm, msg, &allocated_mr);<----- (3)
>     ...
>     }
> 
>     ---
>     int rds_rdma_extra_size(struct rds_rdma_args *args)
>     {
>     ...
>     int tot_pages = 0;
>     ...
>     local_vec = (struct rds_iovec __user *)(unsigned long)
>     args->local_vec_addr;
> 
>     /* figure out the number of pages in the vector */
>     for (i = 0; i < args->nr_local; i++) {<---- (3)
>     if (copy_from_user(&vec, &local_vec[i],
>        sizeof(struct rds_iovec)))
>     return -EFAULT;
> 
>     ...
>     /* skipped */
>     }
> 
>     return tot_pages * sizeof(struct scatterlist); <----- (4) :
>     tot_pages = 0;
>     }
>     ----
>     (1) - rds_rm_size() does some sanity checks and return the number of
>     pages should be allocated for the buffer.
>     (2) - rds_cmsg_send() parse the cmsghdr structure and call the
>     appropriate function to handle the packet
>     (3) - in rds_rm_size() calls rds_rdma_extra_size(), when
>     cmsg->cmsg_types has RDS_CMSG_RDMA_ARGS , args is a user
>     controllable object, these line of codes (in the loop) are never
>     reached when nr_local = 0
>     (4) - if we put nr_local to zero the former function returns 0 due
>     tot_page= 0;
> 
>     int rds_cmsg_rdma_args(struct rds_sock *rs, struct rds_message *rm,
>       struct cmsghdr *cmsg)
>     {
>     ...
> 
>     op->op_sg = rds_message_alloc_sgs(rm, nr_pages); <---- (5)
>     if (!op->op_sg) {
>     ret = -ENOMEM;
>     goto out;
>     }
>     ...
>     ...
>     }
>     ---
> 
>     void sg_init_table(struct scatterlist *sgl, unsigned int nents)
>     {
>     memset(sgl, 0, sizeof(*sgl) * nents);
>     #ifdef CONFIG_DEBUG_SG
>     {
>     unsigned int i;
>     for (i = 0; i < nents; i++)
>     sgl[i].sg_magic = SG_MAGIC;
>     }
>     #endif
>     sg_mark_end(&sgl[nents - 1]);<----- (6)
>     }
> 
>     (5) - rds_message_alloc_sgs() allocates pages for DMA , and keep in
>     mind that nr_pages still has 0
>     (6) - sg_init_table is called from rds_message_alloc_sgs(), it tries
>     to initialize the buffer for the incoming packets, the function
>     doesn't check 'nents' variable (which is nr_pages=0) .
>     by decrementing nents by 1, it gives huge invalid integer (integer
>     underflow) which is used as index in the scatterlist array,
> 
> 
>     I've attached also a proof of concept code that crashes the kernel
>     as well as the panic log
> 
>     Bests,
>     Mohamed
> 
> 
> 
> 
>