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]
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ