netdev - Re: [PATCH V3 net-next 2/3] rds: deliver zerocopy completion notification with data

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <950eb12c-0520-3656-6667-c13148cec326@oracle.com>
Date:   Mon, 26 Feb 2018 09:07:41 -0800
From:   Santosh Shilimkar <santosh.shilimkar@...cle.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     netdev@...r.kernel.org, willemdebruijn.kernel@...il.com,
        davem@...emloft.net, Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH V3 net-next 2/3] rds: deliver zerocopy completion
 notification with data


On 2/25/2018 3:21 PM, Sowmini Varadhan wrote:
> This commit is an optimization over commit 01883eda72bd
> ("rds: support for zcopy completion notification") for PF_RDS sockets.
> 
> RDS applications are predominantly request-response transactions, so
> it is more efficient to reduce the number of system calls and have
> zerocopy completion notification delivered as ancillary data on the
> POLLIN channel.
> 
> Cookies are passed up as ancillary data (at level SOL_RDS) in a
> struct rds_zcopy_cookies when the returned value of recvmsg() is
> greater than, or equal to, 0. A max of RDS_MAX_ZCOOKIES may be passed
> with each message.
> 
> This commit removes support for zerocopy completion notification on
> MSG_ERRQUEUE for PF_RDS sockets.
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@...cle.com>
> ---
> v2: remove sk_error_queue path; lot of cautionary checks rds_recvmsg_zcookie()
>      and callers to make sure we dont remove cookies from the queue and then
>      fail to pass it up to caller
> v3:
>    - bounds check on skb->cb to make sure there is enough room for
>      struct rds_zcopy_cookies as well as the rds_znotifier;
>    - Refactor cautionary checks in rds_recvmsg_zcookie: if no msg_control
>      has been passed, or if there not enough msg_controllen for a
>      a rds_zcopy_cookies, return silently (do not return error, as the
>      caller may have wanted other ancillary data which may happen to fit
>      in the space provided)
>    - return bool form rds_recvmsg_zcookie, some other code cleanup
> 
Just in case you haven't seen yet, Dan Carpenter reported skb deref
warning on previous version of the patch. Not sure why it wasn't sent
on netdev.

smatch warnings:
net/rds/recv.c:605 rds_recvmsg_zcookie() warn: variable dereferenced 
before check 'skb' (see line 596)

With that addressed,

Acked-by: Santosh Shilimkar <santosh.shilimkar@...cle.com>