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;
Date:   Mon, 10 Aug 2020 20:40:47 +0200 (CEST)
From:   Richard Weinberger <richard@nod.at>
To:     Pavel Machek <pavel@denx.de>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        stable <stable@vger.kernel.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        stable <stable@kernel.org>
Message-ID: <1475688016.229129.1597084847149.JavaMail.zimbra@nod.at>
In-Reply-To: <20200810163851.GB24408@amd>
References: <20200810151804.199494191@linuxfoundation.org> <20200810151804.911709325@linuxfoundation.org> <20200810163851.GB24408@amd>
Subject: Re: [PATCH 4.19 14/48] mtd: properly check all write ioctls for
 permissions
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Thread-Topic: properly check all write ioctls for permissions
Thread-Index: SPuvyTwnFDc3qnA8Gvg/tEw1EZFuYQ==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

----- Ursprüngliche Mail -----
> Von: "Pavel Machek" <pavel@denx.de>
>> When doing a "write" ioctl call, properly check that we have permissions
>> to do so before copying anything from userspace or anything else so we
>> can "fail fast".  This includes also covering the MEMWRITE ioctl which
>> previously missed checking for this.
> 
>> +	/* "safe" commands */
>> +	case MEMGETREGIONCOUNT:
> 
> I wonder if MEMSETBADBLOCK, MEMLOCK/MEMUNLOCK, BLKPG, OTPLOCK and
> MTDFILEMODE should be in the list of "safe" commands? Sounds like they
> can do at least as much damage as average MEMWRITE...

Most of the ioctls you listed are not write-exclusive because existing
user space applications (such as mtd-utils) issue them on a read-only fd.
So, we didn't want to break them.
Before we move such an ioctl to the "non-safe" list, common user space needs to
be inspected. This includes, android, openwrt, mtd-utils, etc...

On the other hand, this is a raw mtd, it is hard to draw the line.
For NAND even reading allows an attacker doing harm, she can trigger read-distrurb
super efficiently using the read ioctl...

So passing an mtdchar fd (no matter whether read or write mode) to untrusted
entities is a bad idea.

Thanks,
//richard