linux-kernel - Re: [PATCH 1/5] fs: add netlink notification interface

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 23 Aug 2011 11:03:22 +0200 (CEST)
From:	Lukas Czerner <lczerner@...hat.com>
To:	"Ted Ts'o" <tytso@....edu>
cc:	Lukas Czerner <lczerner@...hat.com>, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH 1/5] fs: add netlink notification interface

On Fri, 19 Aug 2011, Ted Ts'o wrote:

> On Thu, Aug 18, 2011 at 02:18:22PM +0200, Lukas Czerner wrote:
> > +void fs_nl_send_warning(dev_t dev, unsigned int warntype)
> > +{
> 
> I'm pretty sure this interface isn't going to be sufficient in the
> long run.  I can imagine that people might want to pass in a struct
> inode *, or a struct file * if available, so we could pass back the
> inode number involved, or even the pathname.

Ok, it makes sense to extend the interface a bit. So what about:

voif fs_nl_send_warning(dev_t dev, unsigned int warntype,
			unsigned int flags, void *data);

So we can get the type of the warning, flags which will provide more
information about the waring as for example what type of data are we
attaching, or what type of quota is that etc... And with data we can
easily pass struct inode *, or struct file * or whatever the type of
warning needs for better description of the problem.

> 
> Also, I'd suggesting changing "warning" to "error", since that's what
> most of the things we are sending really are...

I am ok with calling it error, but I do not consider ENOSPC, or
exceeding quota to be an error from the kernel point of view. In
application it is of course an error, but in file system it is just the
state of the things. But anyway it is just my viewpoint and if people
think that it should be called *error* i am ok with that.

> 
> > +	ret = nla_put_u32(skb, FS_NL_A_WARNING, warntype);
> > +	if (ret)
> > +		goto attr_err_out;
> 
> Why not make the warning type to be a string instead of an integer?
> This would make it easier to extend the protocol in the future.

I think that having an ID of the type of the message is important so we
can clearly say what happened without the need of parsing the string.
Otherwise it would be no different than printk's.

I am not sure why would we want to pass string instead. If we want to
pass random errors down this interface I do not think that is going to
be very useful.

> 
> > +	ret = nla_put_u64(skb, FS_NL_A_CAUSED_ID, current_uid());
> > +	if (ret)
> > +		goto attr_err_out;
> 
> I'd also suggest passing back the process id that requested the I/O if
> that's known...

Good point.

> 
> 						- Ted
> 

Thanks!
-Lukas
--
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/