Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Stephen Brennan <stephen.s.brennan@oracle.com>
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] namei: fix use-after-free and adjust calling
 conventions
In-Reply-To: <YTfVE7IbbTV71Own@zeniv-ca.linux.org.uk>
References: <20210901175144.121048-1-stephen.s.brennan@oracle.com>
 <YTfVE7IbbTV71Own@zeniv-ca.linux.org.uk>
Date:   Wed, 08 Sep 2021 11:47:59 -0700
Message-ID: <87mtomsy3k.fsf@stepbren-lnx.us.oracle.com>
Content-Type: text/plain
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?fyXstCtChJRPzz4vGSoRX5Vb5HpGKbA4vsELy6cLpMiLv8bO7DSM2GvYx3E3?=
 =?us-ascii?Q?7DnlnGCNPUn4kV6LwDx06vu9Vx6hABivABNFZM0aocLI9CJjnNmqWPLH/zCd?=
 =?us-ascii?Q?mOdoUl1Y2gDtcmNJwJEshLDxx6b4iSvgicq2XmwWjmIT1C3v5zospiHSQMQA?=
 =?us-ascii?Q?XKHK/jZAocMEC/w4wY1vxodSnWWce7CKWVri6OZagjzPiF6eHLcqP4Bvu1Jp?=
 =?us-ascii?Q?Ohjk7yGOPkSxdkzsGGIrt4P1Mrp7AnzTb2KxFXT3BcBDp+klm1dSFzSVIU4v?=
 =?us-ascii?Q?VJ6/nShbDAT/k3OMmpZLiW63dO/iv8/IulZF3BH0MONKR/VNE3quXI4qPHIk?=
 =?us-ascii?Q?pC6rp2Dzb1GBUVRDOcOIgB6GtH8rd+vu09e5Pd+ggNj5+s0tV+RJGQs2yTag?=
 =?us-ascii?Q?dIsBOlzY7XU8Pqkj5oyRR09P82j6uKDGNkwWWuTPnLZlEU8fVDmqB8M0ECSr?=
 =?us-ascii?Q?GKBt+rVE6hlSXJ0LjeCrlZ3dkOcKA/UfbBB7Ezoyv5suqdX9eId9Au5AQEU0?=
 =?us-ascii?Q?OK9MGlqBHZLbdlkuW7XwBFQ3Lq4Wc0bJXHeFHrjJU9bDxgsyKY4/eVv5Uuxs?=
 =?us-ascii?Q?4Yr6oVPC9JCJWN0OGIGeAdl3VCora3PS0PlOFGHUxhepfoJtxwT9VLSVgjZ7?=
 =?us-ascii?Q?79gS3jBaUG3OQA2z5uLHZBHqhj8pYjXXVQXxIu5wLRn/xMZuYKsref0SXxZM?=
 =?us-ascii?Q?kSqMhz0JV8AejDq4gx5cCXjMh11+44CSAjt+ceNQ61MNksRR5TyCt0v+y3qs?=
 =?us-ascii?Q?8c8AOruXKtofdHWFr2viCUxr/D42b0MqaKbSsZjuvPNIAq1oZ62gbEiXON6u?=
 =?us-ascii?Q?eCKizEIyEza3vbWED7HI5qswuNIhS8a+z4W+wVzw7BSCMZL7QunbutKQc3r4?=
 =?us-ascii?Q?pcUzBUj6i61g12lg2hDri00SCy66/6ChcD1f9j7mJqMKLDlaW4QSfm/NtxEC?=
 =?us-ascii?Q?PhASHn4ulRFooDR/DhyVfSxQNRRu6XvJXhi0725pB4rehooJdvpamcZC8k4w?=
 =?us-ascii?Q?7JhIp574r1UFMbEOBywDtaXVWUP34UU/btkjl5uhqx6aMWhRoEmsKqxR32dV?=
 =?us-ascii?Q?GLGMZdiaOItVQjP4ez/8VeQ2kGke9BDWUmthrjfB0wpCpRp3NiR0WARaGzkX?=
 =?us-ascii?Q?MDum1/rzjofrXoRDQwaYagOQTq+NtKWjj4iiZykS8KsqoZ/iqRDcbZ0I/g+2?=
 =?us-ascii?Q?EloczorgCJFmfZR0KU07x19FgbuRaIWs0xxa6ZcldUPFbaC/5yDU67grXG+3?=
 =?us-ascii?Q?wc3sJrH8HpChL5ky2af630vVtb6+oCU/cdq77wEDFGRvXNOczlYfT4q+dzus?=
 =?us-ascii?Q?Uy9O1jag4qTHm+RC+ODyK1Cu?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 474e0d7b-0d02-4ca3-9d3c-08d972f92cff
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4166.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Sep 2021 18:47:57.9855
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 5qE+DeIKoETeW4UDtFPz6mso//+qsXxiYYnIOLVa8TwHFxOXtlXAq1LygEw9oZYYpmQZjtM/dlfdA2N5zwLWqbrd2l0hxULhkyphmHsh0Zc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4986
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10101 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999 bulkscore=0
 suspectscore=0 mlxscore=0 phishscore=0 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109030001
 definitions=main-2109080117
X-Proofpoint-GUID: 8sXFP1yOilDjSTmbvY0rpy3Lx7GcHdCy
X-Proofpoint-ORIG-GUID: 8sXFP1yOilDjSTmbvY0rpy3Lx7GcHdCy
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Al Viro <viro@zeniv.linux.org.uk> writes:
> [snip]
> Another part I really dislike in that area (not your fault, obviously)
> is
>
> void putname(struct filename *name)
> {
>         if (IS_ERR_OR_NULL(name))
> 		return;
>
> in mainline right now.  Could somebody explain when the hell has NULL
> become a possibility here?  OK, I buy putname(ERR_PTR(...)) being
> a no-op, but IME every sodding time we mixed NULL and ERR_PTR() in
> an API we ended up with headache later.
>
> 	IS_ERR_OR_NULL() is almost always wrong.  NULL as argument
> for destructor makes sense when constructor can fail with NULL;
> not the case here.
>
> 	How about the variant in vfs.git#misc.namei?

I went and looked through the changelog of fs/namei.c since this was
changed and don't see anything setting a filename NULL, so it seems safe
and good to me. I couldn't check *every* user of filename but the change
was only two months ago. Feel free to use my r-b for that commit if you
want.

Reviewed-by: Stephen Brennan <stephen.s.brennan@oracle.com>