NetBSD Problem Report #56005

From mrg@eterna.com.au  Mon Feb 22 06:07:24 2021
Return-Path: <mrg@eterna.com.au>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 295041A921F
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 22 Feb 2021 06:07:24 +0000 (UTC)
Message-Id: <20210222060720.8A1F015912E@splode.eterna.com.au>
Date: Mon, 22 Feb 2021 17:07:20 +1100 (AEDT)
From: mrg@eterna.com.au
Reply-To: mrg@eterna.com.au
To: gnats-bugs@NetBSD.org
Subject: compat_10 open(2) vs NULL checks are implemented poorly
X-Send-Pr-Version: 3.95

>Number:         56005
>Category:       kern
>Synopsis:       open(2) vs NULL checks are implemented poorly
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Feb 22 06:10:00 +0000 2021
>Originator:     matthew green
>Release:        8, 9, -current.
>Organization:
people's front against (bozotic) www (softwar foundation)
>Environment:
>Description:

	do_sys_openat() has a hook to call vfs_openat_10_hook if set.
	if the compat_10 module is loaded (either statically, or by
	manually or automatically loading it), then this hook will be
	set and it will allow the smantic of PATH is NULL is the same
	as attempting to open in ".".

	there are two main issues:

	- the check applies to both open(2) and openat(2), but there
	was no openat(2) in netbsd 1.0, so this hok should not be
	used there at all.

	- this check applies to all calls to open(2) or openat(2),
	regardless of whether the binary is from netbsd 1.0 or not.
	ie, -current today, using a pure -current system that has
	compat_10 loaded (again, statically or dynamically), will
	see the different in behaviour with modern binaries.

	another issue, the first block in do_sys_openat could be
	replaced with this:

	if (path == NULL)
		MODULE_HOOK_CALL(vfs_openat_10_hook, (&pb), enosys(), error);
	else
		error = pathbuf_copyin(path, &pb);
	if (error)
		return error;

	the "goto no_compat" will always fail copyinstr as path is
	NULL here, and the error handling above works fine.  but this
	only shuffles around some code, does not fix anything or even
	change anything perhaps but which errno is returned upon a
	specific error condition.

>How-To-Repeat:

	call open(NULL, O_RDONLY) with or without compat_10 loaded
	and observe the different in behaviour.

>Fix:

	i think the only real way is to version both open(2) and
	openat(2).  anything else involves older, but post 1.0,
	binaries having a potentially different semantics than
	they did so far.  (ie, some app for modern netbsd might
	only have been running on a system with compat_10, and
	thus will break if we change the semantics..)

	if we do version these, i'm of a mind that we should keep
	the old entry points in the kernel and not demote them to
	compat-only, at least for a number of years, as having
	open(2) fail always would be one of the worst failure modes
	that we could induce, and proper code sharing would make
	this only a tiny cost.

Home	PR Database Search