netdev - Re: Question on ptr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAF8QhUiX_ba1gavf1yODAyvFyBr6vJ2mgZWsnH53BkKzXY4u2g@mail.gmail.com>
Date:   Wed, 16 Jan 2019 01:10:31 +0800
From:   fei phung <feiphung27@...il.com>
To:     netdev@...r.kernel.org, mst@...hat.com, feiphung@...mail.com
Subject: Re: Question on ptr_ring linux header

Hi,

inline int pop_circ_queue(struct ptr_ring * buffer, struct item * item_pop)
{
        if (!ptr_ring_empty_any(buffer)) // if (not empty)
        {
                DEBUG_MSG(KERN_INFO "Before pop, head = %u , tail =
%u\n", buffer->consumer_head, buffer->consumer_tail);

                /* extract one item struct containing two unsigned
integers from the buffer */
                *item_pop = *((struct item *)ptr_ring_consume_any(buffer));

                DEBUG_MSG(KERN_INFO "val1 = %u , val2 = %u\n",
item_pop->val1, item_pop->val2);

                DEBUG_MSG(KERN_INFO "After pop, head = %u , tail =
%u\n", buffer->consumer_head, buffer->consumer_tail);

                return 0;
        }

        else return 1; // empty, nothing to pop from the ring
}

> https://gist.github.com/promach/65e9331d55a43a2815239430a28e29c6#file-circ_ring-c-L44
> racy if there are multiple consumers.
> just call ptr_ring_consume_any.

> And it seems to leak the memory the pointer to which you
> have consumed - although it's possible it's freed elsewhere -

I will definitely just call ptr_ring_consume_any() without ptr_ring_empty_any()

Which exact line has memory leak ?
Are you referring to   struct item * item_pop   ?




// TX (PC receive) scatter gather buffer is read.
if (vect & (1<<((5*i)+1))) {
        recv = 1;

        item_recv_push[sc->recv[chnl]->msgs->producer].val1 = EVENT_SG_BUF_READ;
        item_recv_push[sc->recv[chnl]->msgs->producer].val2 = 0;

        // Keep track so the thread can handle this.
        if (push_circ_queue(sc->recv[chnl]->msgs,
&item_recv_push[sc->recv[chnl]->msgs->producer])) {
                printk(KERN_ERR "riffa: fpga:%d chnl:%d, recv sg buf
read msg queue full\n", sc->id, chnl);
        }
        DEBUG_MSG(KERN_INFO "riffa: fpga:%d chnl:%d, recv sg buf
read\n", sc->id, chnl);
}

// TX (PC receive) transaction done.
if (vect & (1<<((5*i)+2))) {
        recv = 1;

        item_recv_push[sc->recv[chnl]->msgs->producer].val1 = EVENT_TXN_DONE;
        item_recv_push[sc->recv[chnl]->msgs->producer].val2 = len;

        // Read the transferred amount.
        len = read_reg(sc, CHNL_REG(chnl, TX_TNFR_LEN_REG_OFF));
        // Notify the thread.
        if (push_circ_queue(sc->recv[chnl]->msgs,
&item_recv_push[sc->recv[chnl]->msgs->producer])) {
                printk(KERN_ERR "riffa: fpga:%d chnl:%d, recv txn done
msg queue full\n", sc->id, chnl);
        }
        DEBUG_MSG(KERN_INFO "riffa: fpga:%d chnl:%d, recv txn done\n",
sc->id, chnl);
}

> https://gist.github.com/promach/7716ee8addcaa33fda140d74d1ad94d6#file-riffa_driver-c-L663
> this one seems to poke at the internal producer index for its own
> purposes. That's not something I expected when I built ptr_ring

if I do not use ->producer in the above multiple (due to multiple if()
clause) sequential
push_circ_queue() operations,  what else I can use other than
->producer to store the data
for item_recv_push   ? Probably a simple integer indexing will do.




> https://i.imgur.com/xWJOH1G.png
> I am having problem getting proper ptr_ring operation where one
> ptr_ring entry (val1=2 for item_recv_pop) is missing from the void ** queue

This "ptr_ring packet" drop does not make any sense to me.

Regards,
Phung