linux-kernel - Re: [PATCH] pstore: Fail to unlink if a driver has not defined pstore

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <51C92F76.70507@linux.vnet.ibm.com>
Date:	Tue, 25 Jun 2013 11:19:42 +0530
From:	Aruna Balakrishnaiah <aruna@...ux.vnet.ibm.com>
To:	Kees Cook <keescook@...omium.org>
CC:	Colin Cross <ccross@...roid.com>, Tony Luck <tony.luck@...el.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Anton Vorontsov <cbouatmailru@...il.com>,
	jkenisto@...ux.vnet.ibm.com, benh@...nel.crashing.org,
	ananth@...ibm.com, mahesh@...ux.vnet.ibm.com
Subject: Re: [PATCH] pstore: Fail to unlink if a driver has not defined pstore_erase

Hi Keek,

On Monday 24 June 2013 10:33 PM, Kees Cook wrote:
> On Mon, Jun 24, 2013 at 12:48 AM, Aruna Balakrishnaiah
> <aruna@...ux.vnet.ibm.com> wrote:
>> pstore_erase is used to erase the record from the persistent store.
>> So if a driver has not defined pstore_erase callback return
>> -EINVAL instead of unlinking a file as deleting the file without
>> erasing its record in persistent store will give a wrong impression
>> to customers.
> This is probably true -- I originally liked the idea of being able to
> clean up the entries, regardless of their storage state, but you're
> probably right. They shouldn't be deleted unless they can _actually_
> be deleted.
>
> So, I support this change, but I think the return needs to be
> different. EINVAL isn't listed, for example, in unlink(2)'s man-page.
> Perhaps EROFS, EACCESS, or EPERM?

The filesystem (pstore) has privileges to unlink the file but only if the
callback function is defined. Since the filesystem has privileges I didn't
consider these error codes (EROFS, EACCESS or EPERM).

In the case where callback function is not defined unlinking the file would
be an invalid operation and hence EINVAL.

Since unlink(2) man page does not have EINVAL listed, I feel going with
EPERM will make more sense.


>
> -Kees
>
>> Signed-off-by: Aruna Balakrishnaiah <aruna@...ux.vnet.ibm.com>
>> ---
>>   fs/pstore/inode.c |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
>> index e4bcb2c..fa6339a 100644
>> --- a/fs/pstore/inode.c
>> +++ b/fs/pstore/inode.c
>> @@ -178,6 +178,8 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry)
>>          if (p->psi->erase)
>>                  p->psi->erase(p->type, p->id, p->count,
>>                                dentry->d_inode->i_ctime, p->psi);
>> +       else
>> +               return -EINVAL;
>>
>>          return simple_unlink(dir, dentry);
>>   }
>>
>
>
> --
> Kees Cook
> Chrome OS Security
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/