From owner-svn-src-all@freebsd.org Thu Jun 20 19:43:39 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F414215C601C for ; Thu, 20 Jun 2019 19:43:38 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) 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 90409940EB for ; Thu, 20 Jun 2019 19:43:38 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x82d.google.com with SMTP id m29so4474555qtu.1 for ; Thu, 20 Jun 2019 12:43:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=bAmL+mxpj2j6X1RjOIfgLsc8OX7cYgdA3KugbeEztl4=; b=AF6t9QWjZpuIc+DzX3x2xm3uGX2AHsafrLYMBzvZZ+zAsjnxI4S3pyAFf0njlTxYJf xespNHoh6/AHE9v5kUZHRuKJHkKF2sOqLDm6Fd141C3g7otSrOuuSh8NMFjwb7Hv5lgb 8ugTZgB2qB2koZV6EGPclH1xi3IerWVmerihupZQeFaxgDIG0ocSrFOZp9A5+874+zRn k8cYqZQEIMKaEt4+Pu2TnkZRCOz31HvCWPWV9ZXHXGMFIYaM6Bppq/0LwseqtWP1TncB gcpV0NlgtWBOuyIWflaf8de1mTRckKZRMe/eMYzeNIFhbsnpIwaOG/DH+GbZLwZXjiWC fzDQ== 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=bAmL+mxpj2j6X1RjOIfgLsc8OX7cYgdA3KugbeEztl4=; b=jk5VqON4W6GB1utTEq0Ge0MlAsVJ2evwO8ri1OxxiSItU3UdI5cRpjDGw2jJi37DNU CestVCUfY6Kl4ac651jVAS9v24DbJldlqEdfWphFfYny5Z+jHzOoW6Mho8luS2QfU4w4 Z8eAYjKF+y0cZr8lFVjOCE5C3w2O58MzwTA06lJ+LupM9wixYjCTw/9l0YdJG50d0rxT 7U8gZefPLxTbVHSQA3GFJob4jyQ6e15g6rx1n4i73NUOiDYwHExmEID8DrHw473Qu7Io 43rzHiCmMD+rGfr/F1tgTZPInsBwMKIqeGCyZUrxbseal1hl8Tlv8BFCqVWq+5HmBO3y rlqA== X-Gm-Message-State: APjAAAWMsGqyWnNHOR7Y2ZT+5kKawCsipEqFckt6PiGJOsvpnfn2z5L9 HvFBysdGocOEV9/R6l9SZBwUf30aNSduw0aJLdXnLA== X-Google-Smtp-Source: APXvYqxPen0laQiS8FLg7OTpRunMXQfeAlo6cL4f6Jv1ulo9MENvsTb+fsAJOiusKIYjpMCDNS9i4HyIWC9wYr8YnJU= X-Received: by 2002:ac8:431e:: with SMTP id z30mr113642501qtm.291.1561059817817; Thu, 20 Jun 2019 12:43:37 -0700 (PDT) MIME-Version: 1.0 References: <201906201435.x5KEZTqH021513@repo.freebsd.org> <54f3bc97cbb485cdcc44b81c82c149ac9e46d42f.camel@freebsd.org> <20190621013236.N5105@besplex.bde.org> <20190621033049.D5823@besplex.bde.org> In-Reply-To: <20190621033049.D5823@besplex.bde.org> From: Warner Losh Date: Thu, 20 Jun 2019 12:43:26 -0700 Message-ID: Subject: Re: svn commit: r349233 - head/sys/sys To: Bruce Evans Cc: Alan Somers , Ian Lepore , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 90409940EB X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.97 / 15.00]; REPLY(-4.00)[]; NEURAL_HAM_SHORT(-0.97)[-0.968,0]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jun 2019 19:43:39 -0000 On Thu, Jun 20, 2019, 11:44 AM Bruce Evans wrote: > On Thu, 20 Jun 2019, Alan Somers wrote: > > > On Thu, Jun 20, 2019 at 10:43 AM Bruce Evans > wrote: > >> Summary: and the headers that it includes should declare > >> minimal types to compile (so __int64_t is enough). Most uses of this > >> header require including domain-specific headers which declare the > >> relevant data structures. > > > > Bruce, would you be satisfied by switching from to > > and from int64_t to __int64_t? > > Not quite. The kernel block number type is daddr_t, and [__]int64_t is > a hard coding of that. Hard-coding of typedefs is good for reducing > namespace pollution, but it is not done for the nearby use of off_t. > > Unfortunately, daddr_t is only declared in . > > The type daddr_t seems to be correct, but ffs conflates it with > ufs2_daddr_t. It is only for blocks of size DEV_BSIZE, while fs blocks > are usually larger. The conflation is just a style bug because 64 bits > should be large enough for anyone and ffs does the necessary conversions > between DEV_BSIZE-blocks and fs-blocks. > > ext2fs seems to be more broken in this area. It doesn't have > ext2fs_daddr_t, but hard-codes this as int32_t or int64_t. int32_t > became very broken when ext2fs started attempting to support unsigned > block numbers. Now, ext2_bmap() corrupts "daddr_t a_bn" to "int32_t > bn" when it calls ext4_bmapext(). This has a chance of working via > unsign-extension bugs provided a_bn fits in uint64_t. It is sure to > fit for a valid a_bn, but perhaps your new ioctl allows probing for > panics using invalid a_bn. ext2_bmap() also calls ext2_bmaparray(). > That used to have essentially the same style bugs as ffs (with the > ext2fs type corresponding to ufs2_daddr_t spelled int32_t), but it now > correctly uses daddr_t. Internally, it misuses daddr_t for ext2fs > block numbers, and there is an ext2fs block number type after all > (e2fs_lbn_t = int64_t) which it uses only for metalbn. > > It looks like the older ext2fs code was fixed, especially if older ext2fs > is limited to int32_t block numbers, but the ext4 case is quite broken > since unsign extension bugs don't help it. a_bn starts as int64_t, then > is truncated to the function arg "int32_t bn", then the function assigns bn > to "int64_t lbn" and doesn't use bn again. > > Using a generic int64_t type in all interfaces would avoid some of these > bugs, so I don't mind using it for the API. Just add a note that it must > be large enough to represent all useful values of daddr_t. > Maybe we should add a __daddr_t define to sys/_types.h? And the usual reshuffling. That would also fix the namespace pollution. Warner >