Re: [PATCH V2 net-next 4/7] rds: support for zcopy completion notification

On Wed, Feb 14, 2018 at 5:28 AM, Sowmini Varadhan
[off-list ref] wrote:

RDS removes a datagram (rds_message) from the retransmit queue when
an ACK is received. The ACK indicates that the receiver has queued
the RDS datagram, so that the sender can safely forget the datagram.
When all references to the rds_message are quiesced, rds_message_purge
is called to release resources used by the rds_message

If the datagram to be removed had pinned pages set up, add
an entry to the rs->rs_znotify_queue so that the notifcation
will be sent up via rds_rm_zerocopy_callback() when the
rds_message is eventually freed by rds_message_purge.

rds_rm_zerocopy_callback() attempts to batch the number of cookies
sent with each notification  to a max of SO_EE_ORIGIN_MAX_ZCOOKIES.
This is achieved by checking the tail skb in the sk_error_queue:
if this has room for one more cookie, the cookie from the
current notification is added; else a new skb is added to the
sk_error_queue. Every invocation of rds_rm_zerocopy_callback() will
trigger a ->sk_error_report to notify the application.

Signed-off-by: Sowmini Varadhan <redacted>
---

+static void rds_rm_zerocopy_callback(struct rds_sock *rs,
+                                    struct rds_znotifier *znotif)
+{
+       struct sock *sk = rds_rs_to_sk(rs);
+       struct sk_buff *skb, *tail;
+       struct sock_exterr_skb *serr;
+       unsigned long flags;
+       struct sk_buff_head *q;
+       u32 cookie = znotif->z_cookie;
+
+       q = &sk->sk_error_queue;
+       spin_lock_irqsave(&q->lock, flags);
+       tail = skb_peek_tail(q);
+
+       if (tail && skb_zcookie_add(tail, cookie)) {
+               spin_unlock_irqrestore(&q->lock, flags);
+               mm_unaccount_pinned_pages(&znotif->z_mmp);
+               consume_skb(rds_skb_from_znotifier(znotif));
+               sk->sk_error_report(sk);
+               return;
+       }
+
+       skb = rds_skb_from_znotifier(znotif);
+       serr = SKB_EXT_ERR(skb);
+       serr->ee.ee_errno = 0;
+       serr->ee.ee_origin = SO_EE_ORIGIN_ZCOOKIE;
+       serr->ee.ee_info = 0;
+       serr->ee.ee_code |= SO_EE_CODE_ZEROCOPY_COPIED;

This resets selective fields in skb->cb[]. That array was previously used
as struct rds_znotifier, so may have non-zero state left over. Due to length
of serr->header it is likely that notifier and serr->ee do not overlap. But if
they do kernel data is leaked to userspace. And size of rds_znotifier may
change, so relying on that is fragile. Safer to clear serr->ee completely.