From owner-freebsd-current@freebsd.org Mon Sep 14 07:45:06 2020 Return-Path: Delivered-To: freebsd-current@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 83E453F5C74 for ; Mon, 14 Sep 2020 07:45:06 +0000 (UTC) (envelope-from delphij@gmail.com) Received: from mail-il1-x143.google.com (mail-il1-x143.google.com [IPv6:2607:f8b0:4864:20::143]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BqdgP4LH2z4M6v; Mon, 14 Sep 2020 07:45:05 +0000 (UTC) (envelope-from delphij@gmail.com) Received: by mail-il1-x143.google.com with SMTP id t13so15770776ile.9; Mon, 14 Sep 2020 00:45:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=plDdH65KHkSDtDgiSY2XK+n7roVIGgz5vXx5TRPWtnc=; b=LXBhZRGM/XyeDYIY0A+xp8FL+j+tjSt8BG6uW/8n4UzP5e34TpV47eISQoP2Mi92bK hkzJCb/zBj0xp5YkhMoXXoGSQuM/+wkGXpREGjra/+7FQBQh1TsW7Gsr0soDrCTED6Wc 5K9QNzCUCwsrcMIrLVUXWMSmmJlxBvznUZt22znyobss+1h+wdM8tx71wlI21SrprhsK Na+cU9f1PKi2P38bd1ePefh1gIHVGKwhkoR0i1Zh09upCoN2gHpjHLEasboMSfBcnmoN hyudrjN/dQ05i9FR1yPXFmk+IpUHjxxqUNptBljA3V6/S0wGhs/wEEd3IMRzAFbvKTpA 5rKA== X-Gm-Message-State: AOAM5338SVxkk72PZytSNOI6TICUPMQh/B/YVXeCOmsFtoL9kMwFrfDB Sgc6PCnmtAtoQKsogs63+sS3Zy7YoxDgHsnw0XrE5XOcPE7XBg== X-Google-Smtp-Source: ABdhPJwgocDLUIFA6xOx6BK4gGLWgYG1lkU2GyG4/exugqgO5fVHgmVlKb0hya/6a2+NLBX5uAmxeKjPlDx1jBOepBc= X-Received: by 2002:a92:194b:: with SMTP id e11mr11508927ilm.133.1600069502879; Mon, 14 Sep 2020 00:45:02 -0700 (PDT) MIME-Version: 1.0 References: <202004151320.03FDKqT7027080@repo.freebsd.org> In-Reply-To: <202004151320.03FDKqT7027080@repo.freebsd.org> From: Xin LI Date: Mon, 14 Sep 2020 00:44:51 -0700 Message-ID: Subject: ioctl argument type [Was Re: svn commit: r359968 - head/sys/kern] To: FreeBSD Current Cc: hselasky@freebsd.org, phk@freebsd.org, jilles@freebsd.org, Doug Rabson , Xin Li X-Rspamd-Queue-Id: 4BqdgP4LH2z4M6v X-Spamd-Bar: --- X-Spamd-Result: default: False [-3.76 / 15.00]; RCVD_TLS_ALL(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20161025]; FREEFALL_USER(0.00)[delphij]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; NEURAL_HAM_LONG(-1.03)[-1.031]; RCPT_COUNT_FIVE(0.00)[6]; TO_MATCH_ENVRCPT_SOME(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::143:from]; NEURAL_HAM_SHORT(-0.73)[-0.727]; NEURAL_HAM_MEDIUM(-1.01)[-1.005]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_COUNT_TWO(0.00)[2]; MAILMAN_DEST(0.00)[freebsd-current]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.33 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Sep 2020 07:45:06 -0000 Hi, I have seen Chromium trigger the warning (I run -CURRENT with INVARIANTS) and looked into the code history a little bit. It seems that the command was changed to u_long in r36846 with a follow up commit of r38517 , possibly because ioctl was defined to take an unsigned long command before FreeBSD. Internally, we have truncated it to 32-bit since 2005 (r140406 ), and this recent change made it a silent behavior. POSIX, on the other hand, defined ioctl as taking an int as its second parameter, but neither Linux (glibc in particular, despite its documentation says differently) nor macOS appear to define it that way, but Solaris seems to be defining it as an int. What was the motivation to keep the prototype definition as int ioctl(int fd, unsigned long request, ...); instead of: int ioctl(int fd, int request, ...); Other than to make existing code happy? Alternatively, will it be a good idea to give compiler some hints (e.g. by using __attribute__(enable_if)) to emit errors, if we insist keeping the existing signature? On Wed, Apr 15, 2020 at 6:21 AM Hans Petter Selasky wrote: > Author: hselasky > Date: Wed Apr 15 13:20:51 2020 > New Revision: 359968 > URL: https://svnweb.freebsd.org/changeset/base/359968 > > Log: > Cast all ioctl command arguments through uint32_t internally. > > Hide debug print showing use of sign extended ioctl command argument > under INVARIANTS. The print is available to all and can easily fill > up the logs. > > No functional change intended. > > MFC after: 1 week > Sponsored by: Mellanox Technologies > > Modified: > head/sys/kern/sys_generic.c > > Modified: head/sys/kern/sys_generic.c > > ============================================================================== > --- head/sys/kern/sys_generic.c Wed Apr 15 13:13:46 2020 (r359967) > +++ head/sys/kern/sys_generic.c Wed Apr 15 13:20:51 2020 (r359968) > @@ -652,18 +652,19 @@ int > sys_ioctl(struct thread *td, struct ioctl_args *uap) > { > u_char smalldata[SYS_IOCTL_SMALL_SIZE] > __aligned(SYS_IOCTL_SMALL_ALIGN); > - u_long com; > + uint32_t com; > int arg, error; > u_int size; > caddr_t data; > > +#ifdef INVARIANTS > if (uap->com > 0xffffffff) { > printf( > "WARNING pid %d (%s): ioctl sign-extension ioctl > %lx\n", > td->td_proc->p_pid, td->td_name, uap->com); > - uap->com &= 0xffffffff; > } > - com = uap->com; > +#endif > + com = (uint32_t)uap->com; > > /* > * Interpret high order word to find amount of data to be >