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
| ||
|
Message-ID: <30125584-BB17-4727-B202-3D661625845D@oracle.com> Date: Wed, 5 Apr 2023 14:25:37 +0000 From: Chuck Lever III <chuck.lever@...cle.com> To: Jakub Kicinski <kuba@...nel.org> CC: Chuck Lever <cel@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Boris Pismenny <borisp@...dia.com>, "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>, "\"kernel-tls-handshake@...ts.linux.dev\\\" <kernel-tls-handshake@...ts.linux.dev>, John Haxby <john.haxby@...cle.com>\"@mx0b-00069f01.pphosted.com" <"kernel-tls-handshake@...ts.linux.dev\" <kernel-tls-handshake@...ts.linux.dev>, John Haxby <john.haxby@...cle.com>"@mx0b-00069f01.pphosted.com> Subject: Re: [PATCH v8 1/4] net/handshake: Create a NETLINK service for handling handshake requests > On Apr 4, 2023, at 9:32 PM, Jakub Kicinski <kuba@...nel.org> wrote: > > On Wed, 5 Apr 2023 00:44:11 +0000 Chuck Lever III wrote: >>> On Mon, 03 Apr 2023 14:46:02 -0400 Chuck Lever wrote: >>>> +int handshake_nl_done_doit(struct sk_buff *skb, struct genl_info *info) >>>> +{ >>>> + struct net *net = sock_net(skb->sk); >>>> + struct socket *sock = NULL; >>>> + struct handshake_req *req; >>>> + int fd, status, err; >>>> + >>>> + if (GENL_REQ_ATTR_CHECK(info, HANDSHAKE_A_DONE_SOCKFD)) >>>> + return -EINVAL; >>>> + fd = nla_get_u32(info->attrs[HANDSHAKE_A_DONE_SOCKFD]); >>>> + >>>> + err = 0; >>>> + sock = sockfd_lookup(fd, &err); >>>> + if (err) { >>>> + err = -EBADF; >>>> + goto out_status; >>>> + } >>>> + >>>> + req = handshake_req_hash_lookup(sock->sk); >>>> + if (!req) { >>> >>> fput() missing on this path? >> >> fput of ... sock->file? DONE shouldn't do that if >> it can't find the sock's matching handshake_req. > > Hm, sounds odd. sockfd_lookup() returns a sock with incremented > reference count, so if user space passes a random fd unrelated > to any real request - we'll bump the refcount of that fd and > return an error. So the related file is going to get leaked. > What did I miss? 🤔️ Makes sense, got it. I'll add something here. >>>> + err = -EBUSY; >>>> + goto out_status; >>>> + } >>>> + >>>> + trace_handshake_cmd_done(net, req, sock->sk, fd); >>>> + >>>> + status = -EIO; >>>> + if (info->attrs[HANDSHAKE_A_DONE_STATUS]) >>>> + status = nla_get_u32(info->attrs[HANDSHAKE_A_DONE_STATUS]); >>>> + >>>> + handshake_complete(req, status, info); >>>> + fput(sock->file); >>>> + return 0; >>>> + >>>> +out_status: >>>> + trace_handshake_cmd_done_err(net, req, sock->sk, err); >>>> + return err; >>>> +} >>> >>>> + /* >>>> + * Arbitrary limit to prevent handshakes that do not make >>>> + * progress from clogging up the system. >>>> + */ >>>> + si_meminfo(&si); >>>> + tmp = si.totalram / (25 * si.mem_unit); >>>> + hn->hn_pending_max = clamp(tmp, 3UL, 25UL); >>> >>> No idea what this does (what's mem_unit?), we'll have to trust you :) >> >> Paolo requested that we link the pending_max limit to >> the memory size of the system. I thought folks would >> be familiar with the si_meminfo() kernel API. >> >> What does it need? A better comment? A different approach? > > I think comment would be good. Are you clamping to 1/25th of all > available pages? The request take up about a page? As the sign sez, it's an arbitrary cap. I just picked a formula that gave about 10 pending entries on my tiny test system. As always, suggestions are welcome. -- Chuck Lever
Powered by blists - more mailing lists