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

[Santosh Shilimkar]

Sorry I didn't go through the details and thought you were hitting the
length OOB issue. Your fix looks valid to me. Would you please post
the patch on net-dev ?


On 1/2/2018 10:32 AM, Mohamed Ghannam wrote:
> Hi,
>
> I believe that my report has nothing to do with that fix, it's 
> completely unrelated,
> I re-checked the proof of concept and still crash the kernel,
>
> The fix should be simple : args->nr_local should be checked against 0
>
> Please re-check and let me know if you want any further information,
>
> Bests,
> Mohamed
>
> 2018-01-02 17:38 GMT+00:00 Santosh Shilimkar 
> <santosh.shilimkar at oracle.com <mailto:santosh.shilimkar at oracle.com>>:
>
>     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/
>     <https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_10128783_&d=DwMFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=hWpFvp_cTkkwMMULcvbV65orOO9Gv3OUaY0ATWhQwak&m=IO0CDsTpgmJIRZcvXV9IivkHeVmf5fAVxPittvU0VvM&s=AGrRlGToeJ8fptgH2R7ilm4_omqi3dBtHnf4oJBPl3o&e=>
>
>     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>
>         <mailto: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
>
>
>
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/rds-devel/attachments/20180102/d201a73f/attachment.html