From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161766AbcE3QuT (ORCPT ); Mon, 30 May 2016 12:50:19 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:47150 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161626AbcE3QuR (ORCPT ); Mon, 30 May 2016 12:50:17 -0400 Date: Mon, 30 May 2016 17:50:07 +0100 From: Al Viro To: Miklos Szeredi Cc: Krisztian Litkey , Mimi Zohar , linux-unionfs@vger.kernel.org, Krisztian Litkey , linux-security-module , linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 1/1] ovl: setxattr: don't deadlock when called from ima_fix_xattr. Message-ID: <20160530165007.GA14480@ZenIV.linux.org.uk> References: <1463611500.2465.22.camel@linux.vnet.ibm.com> <1463725718-2461-1-git-send-email-kli@iki.fi> <1463754087.2763.17.camel@linux.vnet.ibm.com> <20160520162937.GU14480@ZenIV.linux.org.uk> <1463763633.2763.34.camel@linux.vnet.ibm.com> <20160530141015.GC5758@veci.piliscsaba.szeredi.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160530141015.GC5758@veci.piliscsaba.szeredi.hu> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Only tangentially related, but... that bug had been discussed, without any results: the fallback in ima_d_path() to ->d_name.name is completely broken. There is no warranty whatsoever that dentry won't be renamed, with its earlier (too long to be embedded into dentry itself) ->d_name.name getting freed. Right under your code. Could we please get rid of that thing? "A message in a near-oom situation might be less informative than we'd like" is better than "this code might end up dereferencing freed memory". Another similar bug is in ima_collect_measurement() - const char *filename = file->f_path.dentry->d_name.name; ... integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, "collect_data", audit_cause, with no warranty whatsoever that you are not passing a pointer to freed memory. The same goes for ima_eventname_init_common() - if (event_data->file) { cur_filename = event_data->file->f_path.dentry->d_name.name; cur_filename_len = strlen(cur_filename); } else ... return ima_write_template_field_data(cur_filename, cur_filename_len, DATA_FMT_STRING, field_data);