Received-SPF: pass (google.com: domain of linux-kernel+bounces-77500-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 22 Feb 2024 15:15:17 -0800
From: Dan Williams <dan.j.williams@intel.com>
To: Dan Williams <dan.j.williams@intel.com>, <gregkh@linuxfoundation.org>
CC: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>, Marc Herbert
	<marc.herbert@intel.com>, "Rafael J. Wysocki" <rafael@kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>,
	<linux-coco@lists.linux.dev>, <alsa-devel@alsa-project.org>
Subject: RE: [PATCH 1/3] sysfs: Fix crash on empty group attributes array
Message-ID: <65d7d5858bb74_ca001294af@dwillia2-xfh.jf.intel.com.notmuch>
References: <170863444851.1479840.10249410842428140526.stgit@dwillia2-xfh.jf.intel.com>
 <170863445442.1479840.1818801787239831650.stgit@dwillia2-xfh.jf.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <170863445442.1479840.1818801787239831650.stgit@dwillia2-xfh.jf.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?LOB0e6JHw3pmrhEW8ZuoBlJJklnROxwYjxOyp/Squ/qH0v+0cxHGO0ZZB+wV?=
 =?us-ascii?Q?t56s4vRjrBjRQxC+rbhsWd8Z1JOwqm+2Nd2MFvP/jxMwIBYm0oN3fKicicPh?=
 =?us-ascii?Q?jACFkS8Km7HCfq6czRvtssoAxerLJVV3iJaZY2izYsusxTb+luD6FXW+AW0v?=
 =?us-ascii?Q?cnrlUI5KpuPAG3m46eNewLlk3VJUdE973Ous4N3HJUTvxqQp46Vt8zLTIbDp?=
 =?us-ascii?Q?AXDEndDv3eizfZk7xrRBD8HUUDM/pnc/wdKdChd+yyzD36tzzTt3xPHLsT5b?=
 =?us-ascii?Q?tvJq9T1pv8+dtbgQbQO1etTrT6XbLDMvZ9mjCqLfvO/tmCDtXe+VQVGUq22B?=
 =?us-ascii?Q?v0ri7Q+nyrCkDq1LUxxNVkqBmvJOFxJF0UPhEcg6vT1UW7Vv0o7vPwEgUuAK?=
 =?us-ascii?Q?okx3cZqLfMpCg8QJmxpNhcxX6ISCnLbnFa6bqGFnfedk+zdk20ud8ULlhmHr?=
 =?us-ascii?Q?aump8guUr7JMNtyvJ0y9HGrXyMOVgpgV7q5KiyYm1uLhrX8UQPUwRQiLo7Mo?=
 =?us-ascii?Q?QxI+wD5MVDWxB/BewNKwH5F+uDVnH7vxw1aYSoBBdadhjiBeYoIrVrNgJecZ?=
 =?us-ascii?Q?B0PG2N+GLUGpUU+ZcbK4/36ueLXFz96qtHw+bxup+mm5rnN0c3TpaJbRR4Zz?=
 =?us-ascii?Q?1kyuS0+W66eKESyr+6VSo+DimdwzZQkpmr8uGUsbanUhlEl42l+kiCbw7Bd9?=
 =?us-ascii?Q?RjX+Dyxoo8iWRxCOIUsjatacXKx4CEXxWyP6+oXOI1W2PUrhLJkkahYeoAP4?=
 =?us-ascii?Q?WmLLIu4kwJZrRFIBcMtK71XxW0xFFOaZ24tKzADVUytipuKBXhkMJwTGg84E?=
 =?us-ascii?Q?Kh5zN5N5J4TdOtUw6z9J+8ib5PVYQ85h+aOCTlv+eYhFXg7N7SjFAjpONuQv?=
 =?us-ascii?Q?LIx6wBtpmANsAkZ32w5WIkWnvPwMs8qcsU86ylT2dgJEtu1nsgdJABvKTKs+?=
 =?us-ascii?Q?4zvXLdGTjJhSDhLbYdZ8IArPx9g8t3OFO1n8rDSexcLf7jk9VcmuPX0/vjhV?=
 =?us-ascii?Q?RDRUhYF8CUCXSaz3GRtMV7AgptI7MzR725WKQY0wMGGzNC6xU8ffhWB8F53o?=
 =?us-ascii?Q?rx+e8K/lMp5snzQ3AvuRDgOLfddr+9uZ+o2s2/QX8b2nfQakbFr8bGMb9tgq?=
 =?us-ascii?Q?UPow7xx6mJOaS8ld/PYhnh/q1ZKoS4FTzXMXHo+M3tXGw7AuE/zL57GKW9Hq?=
 =?us-ascii?Q?D5mhbyn0XLMF0oTMPQ1tW02Az+Qb0r4nmv5zYw0wUDzIcH7I8SQBVs8mE7u1?=
 =?us-ascii?Q?VnKSIMj09Ih6baQcWyVR+NDusYyzwiQaF6ag1LT8bGag6sSAR8rRVq+MunoE?=
 =?us-ascii?Q?LtsZkopuNEn1dIGnOKmTXmiex/NaODkD7KEQ5G+WC6zHg7YNo+HKe18Bic07?=
 =?us-ascii?Q?CjxNl/c2W3dsoiUMNoyu0iygk+k28fywM9rOUAx/q6Yek35UMM75aFiKNTBm?=
 =?us-ascii?Q?AsjJxwLH3eGoVg5hr1JoYEKW02f7ahAGj2LYj1rbafcw0Ioq8Y7CbOhyv8Xf?=
 =?us-ascii?Q?ig+zWHLSK2/7EvQ2d4hQQJd5bQw20VoAOu9Tjij1vzUSqx18As07T6rCVmba?=
 =?us-ascii?Q?ZrSQbWLn9m/+zKBvAdMPINsqYBZXp2qtrfeq17q/x/zByW+kiacXsBuH2AJJ?=
 =?us-ascii?Q?ug=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3ed41a4e-3ac2-4737-ea4b-08dc33fc2355
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Feb 2024 23:15:19.9029
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Eib7vnjiESYCgZJM3WKRa+Q2VaMVRRT1SFNyObentWpagRbV5SOq2HFvynba2PO+MdhwW9vW2hZMuegvDIQDjEZdTAifCO795kBBOjdjp28=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB6717
X-OriginatorOrg: intel.com

Dan Williams wrote:
> It turns out that arch/x86/events/intel/core.c makes use of "empty"
> attributes.
> 
> 	static struct attribute *empty_attrs;
> 
> 	__init int intel_pmu_init(void)
> 	{
> 	        struct attribute **extra_skl_attr = &empty_attrs;
> 	        struct attribute **extra_attr = &empty_attrs;
> 	        struct attribute **td_attr    = &empty_attrs;
> 	        struct attribute **mem_attr   = &empty_attrs;
> 	        struct attribute **tsx_attr   = &empty_attrs;
> 		...
> 
> That breaks the assumption __first_visible() that expects that if
> grp->attrs is set then grp->attrs[0] must also be set and results in
> backtraces like:
> 
>     BUG: kernel NULL pointer dereference, address: 00rnel mode
>     #PF: error_code(0x0000) - not-present ] PREEMPT SMP NOPTI
>     CPU: 1 PID: 1 Comm: swapper/IP: 0010:exra_is_visible+0x14/0x20
>      ? exc_page_fault+0x68/0x190
>      internal_create_groups+0x42/0xa0
>      pmu_dev_alloc+0xc0/0xe0
>      perf_event_sysfs_init+0x580000000000 ]---
>     RIP: 0010:exra_is_visible+0x14/0
> 
> Check for non-empty attributes array before calling is_visible().
> 
> Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Closes: https://github.com/thesofproject/linux/pull/4799#issuecomment-1958537212
> Fixes: 70317fd24b41 ("sysfs: Introduce a mechanism to hide static attribute_groups")
> Cc: Marc Herbert <marc.herbert@intel.com>

Ok, this one is now

Tested-by: Marc Herbert <marc.herbert@intel.com>

..per:

https://github.com/thesofproject/linux/pull/4799#issuecomment-1960469389

Thanks for all the help Marc!