From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D259E784A8 for ; Mon, 2 Oct 2023 03:22:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D60096B0152; Sun, 1 Oct 2023 23:22:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D10356B0155; Sun, 1 Oct 2023 23:22:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BFFC66B0185; Sun, 1 Oct 2023 23:22:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id B0DAC6B0152 for ; Sun, 1 Oct 2023 23:22:49 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 7DCA61A02C4 for ; Mon, 2 Oct 2023 03:22:49 +0000 (UTC) X-FDA: 81299074458.28.DB18C7C Received: from out-204.mta0.migadu.com (out-204.mta0.migadu.com [91.218.175.204]) by imf27.hostedemail.com (Postfix) with ESMTP id ABA3540017 for ; Mon, 2 Oct 2023 03:22:47 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=fFCaLqAA; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf27.hostedemail.com: domain of kent.overstreet@linux.dev designates 91.218.175.204 as permitted sender) smtp.mailfrom=kent.overstreet@linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1696216968; a=rsa-sha256; cv=none; b=jMgW3ytrnaayDyAR0osusVl0sUgoW+ThYMg0EkLCSgk/b/E3mlq3n3kiRTAz7THKqBiPdp hwuNr+VZDn64duSKo0T1HEr6bvzJ8/swXFCMSB3JiNctdbWE6lUBl2No/b9ymAiIKnwCda p79DFyTxlhuztO9IaoxLpEmN4Kdsqko= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=fFCaLqAA; dmarc=pass (policy=none) header.from=linux.dev; spf=pass (imf27.hostedemail.com: domain of kent.overstreet@linux.dev designates 91.218.175.204 as permitted sender) smtp.mailfrom=kent.overstreet@linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1696216968; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=JklXW6Z7ajflSgg9Kh9ih47eyxtCv9eOtR0Gj32NGWI=; b=pYw40xIrhW3rUBFn/LG1gtaMWKgP+4YiaTZsosP+d+LDZxvWuxRP/+7rXaxcTqw0tk4v9Q gwlZVssXPNm5LTxmJ+cwvEatHloHhT/whGqoO3Zvv/Pp8IuDAElPxsgdqd/jgRRmMH1Vg5 3FHg+y0ccdPOjCQn4DeprGVW3HKwiG4= Date: Sun, 1 Oct 2023 23:22:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1696216963; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=JklXW6Z7ajflSgg9Kh9ih47eyxtCv9eOtR0Gj32NGWI=; b=fFCaLqAAMWQH/QRpOQ9JxdRKCdWSnR0t7DWuPHFx8mbxDlr7/dAk1mrNvBTjYQM7HMs+FQ Q0Jj7htwUxRj0iB46vCoQPvdomL2lLVdPkoYS2xix3IwFtRDbdbtqNEZco+V+UxChHzKRN a2/2B6psnFUXzoYq6y1jxtvPv25XD58= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Kent Overstreet To: Kees Cook Cc: Andrew Morton , Brian Foster , kernel test robot , Linux Memory Management List , linux-fsdevel@vger.kernel.org, linux-bcachefs@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [linux-next:master] BUILD REGRESSION df964ce9ef9fea10cf131bf6bad8658fde7956f6 Message-ID: <20231002032239.t7ghpigbq5jy3ng7@moria.home.lan> References: <202309301308.d22sJdaF-lkp@intel.com> <202309301403.82201B0A@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <202309301403.82201B0A@keescook> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: ABA3540017 X-Stat-Signature: ctwx4cs46mqgjacr6sdphezietqzjbyp X-HE-Tag: 1696216967-729825 X-HE-Meta: U2FsdGVkX1+3Pn08zlc4kS9yjr0GnW+eNxGKzzf3wfftSmBYLMGlmUNdmTsNHK/l6wLn9RyIkgkt1gwHxX+kFb8YurDGxzzP9C+As1iSAahWHgPmVRI/L1OVJmzOqme0UJZDNJq16W+m0u5W37wv14asGQEVdQpXIQW6yJoQZloOUrzmFGrce8zReqohskcyBztDYwerT3x5j/2xspZJf56ZaAfdMpQY/kflHRjxu4cHVPdhYS8eSAUBEHEAdJtqrZNcjAJA8RDAsqaJmB3x2G25VGjxzib73FHsg9uFun0xXEGLPO9/C0UCyYSrABk+Um09XjY4WzINH92YzQ09pbhL0b4zUZKdMYhfWb7AUkEZq8MiVE+QJPFQM9vU/UCkXs4QzJg0j9I4k2i0VtqxoEXx27Kkm5VIXoSOw5NoIm+4FwS/5FBxCJHHAgKKJx3ConrvFoLfiUoTEqpeiBjgCl5r2SGWogBOfWtS0C6Yv7sR0Bpc20Itdj0hAQVyXCFIUe920spOb1VHpRV8K7/KjwMXpJ3bSpuJ6b6s+1sG8GV5JaSm3K9bK9qGb8NJ5R+4PwYyi+AekSGVB1194Kt5aFB2mYCjdD+oVSAJwU5KB+uSoNSG2QcV6IGQBIogzfB4Jy0jzcYBsOCH9j26naVLbM/3MBwHFofGFtLHiS+k7lKzkt0JYvpOsUMBzJLOJ/TUdZID+5zWYsasdphvFJANrns4kqQcVBPNmNg0iNIRlchyOHYaTVrzclxLShXbEnitLULqRN1xXjzT7jn5tEqnlNSXheCFTjR9onaqLkg80InqjOhgCnZzl0akcAt4TY/7VOgdVB1OeQsRU5wZQFDiMBnAiYouusgSPYlFvUkRgkNeSGR7c/6GdVUhPpMljh2hdR7aCgtH2bhTZ5u4J7y+eYJgGxaOKGxSa6WOsmvDq/GGMgazy60nNxQIlrN0FQYBHYui222J2bT1rVpN6PK 7mBYmsJH wdYia3zKhbIYH+IkA5fIyvaCBuwYT8oK0D7Qrnl8t6gbv1oPM27IitKL53c0w+uSeglCgZStny+koMyrUk0ifiSPVsdRVesOOkBWqdYpMyd9Kyb9cztX5CPMzQ9IhOIMUs+in X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Sep 30, 2023 at 02:56:01PM -0700, Kees Cook wrote: > Hi Kent, > > Andrew pointed this out to me, and it's a FORTIFY issue under a W=1 build: > > On Sat, Sep 30, 2023 at 01:25:34PM +0800, kernel test robot wrote: > > tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > branch HEAD: df964ce9ef9fea10cf131bf6bad8658fde7956f6 Add linux-next specific files for 20230929 > > > > Error/Warning reports: > > > > [...] > > https://lore.kernel.org/oe-kbuild-all/202309192314.VBsjiIm5-lkp@intel.com > > fs/bcachefs/extents.c: In function 'bch2_bkey_append_ptr': > include/linux/fortify-string.h:57:33: warning: writing 8 bytes into a region of size 0 [-Wstringop-overflow=] > 57 | #define __underlying_memcpy __builtin_memcpy > | ^ > include/linux/fortify-string.h:648:9: note: in expansion of macro '__underlying_memcpy' > 648 | __underlying_##op(p, q, __fortify_size); \ > | ^~~~~~~~~~~~~ > include/linux/fortify-string.h:693:26: note: in expansion of macro '__fortify_memcpy_chk' > 693 | #define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \ > | ^~~~~~~~~~~~~~~~~~~~ > fs/bcachefs/extents.c:235:17: note: in expansion of macro 'memcpy' > 235 | memcpy((void *) &k->v + bkey_val_bytes(&k->k), > | ^~~~~~ > fs/bcachefs/bcachefs_format.h:287:33: note: destination object 'v' of size 0 > 287 | struct bch_val v; > | ^ > > The problem here is the struct bch_val is explicitly declared as a > zero-sized array, so the compiler becomes unhappy. :) Converting bch_val > to a flexible array will just kick the can down the road, since this is > going to run into -Wflex-array-member-not-at-end soon too since bch_val > overlaps with other structures: > > struct bch_inode_v3 { > struct bch_val v; > > __le64 bi_journal_seq; > ... > }; > > As a container_of() target, this is fine -- leave it a zero-sized > array. The problem is using it as a destination for memcpy, etc, since > the compiler will believe it to be 0 sized. Instead, we need to impart > a type of some kind so that the compiler can actually unambiguously > reason about sizes. The memcpy() in the warning is targeting bch_val, > so I think the best fix is to correctly handle the different types. > > So just to have everything in front of me, here's a summary of what I'm > seeing in the code: > > struct bkey { > /* Size of combined key and value, in u64s */ > __u8 u64s; > ... > }; > > /* Empty placeholder struct, for container_of() */ > struct bch_val { > __u64 __nothing[0]; > }; > > struct bkey_i { > __u64 _data[0]; > > struct bkey k; > struct bch_val v; > }; > > static inline void bch2_bkey_append_ptr(struct bkey_i *k, struct bch_extent_ptr ptr) > { > EBUG_ON(bch2_bkey_has_device(bkey_i_to_s(k), ptr.dev)); > > switch (k->k.type) { > case KEY_TYPE_btree_ptr: > case KEY_TYPE_btree_ptr_v2: > case KEY_TYPE_extent: > EBUG_ON(bkey_val_u64s(&k->k) >= BKEY_EXTENT_VAL_U64s_MAX); > > ptr.type = 1 << BCH_EXTENT_ENTRY_ptr; > > memcpy((void *) &k->v + bkey_val_bytes(&k->k), > &ptr, > sizeof(ptr)); > k->k.u64s++; > break; > default: > BUG(); > } > } > > So this is appending u64s into the region that start with bkey_i. Could > this gain a u64 flexible array? > > struct bkey_i { > __u64 _data[0]; > > struct bkey k; > struct bch_val v; > __u64 ptrs[]; > }; > > Then the memcpy() could be just a direct assignment: > > k->ptrs[bkey_val_u64s(&k->k)] = (u64)ptr; > k->k.u64s++; No, that's not going to work. You're adding a field that's specific to bch_extent (and not even correct for that) to bkey_i, the generic key; there are many other different types of values. > Alternatively, perhaps struct bkey could be the one to grow this flexible > array, and then it could eventually be tracked with __counted_by (but > not today since it expects to count the array element count, not a whole > struct size): > > struct bkey { > /* Size of combined key and value, in u64s */ > __u8 u64s; > ... > __u64 data[] __counted_by(.u64s - BKEY_U64s); > }; > > And bch_val could be dropped... bch_val can't be dropped. bkey_i is different from bkey; bkey_i is a bkey with an inline value, we also have bkey_s and bkey_s_c for a bkey with a split value (and const variation); bch_val is in bkey_i and not bkey because container_of to get to the value is correct for bkey_i, but not bkey. > Then the memcpy becomes: > > k->k.u64s++; > k->k.data[bkey_val_u64s(&k->k)] = (u64)ptr; > > It just seems like there is a lot of work happening that could really > just type casting or unions... Honestly, I think we really just need an escape hatch. Casting to a void pointer clearly isn't it - and this isn't the only issue I'm still seeing with all the recent FORTIFY_SOURCE stuff, and honestly it's been making me tear my hair out. I'm not leaping at the chance to reorganize my fundamental data structures for this. Can we get such an escape hatch?