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, 10 Oct 2006 14:58:08 -0700
From:	Joel Becker <Joel.Becker@...cle.com>
To:	Paul Menage <menage@...gle.com>
Cc:	Chandra Seetharaman <sekharan@...ibm.com>, akpm@...l.org,
	ckrm-tech@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [ckrm-tech] [PATCH 0/5] Allow more than PAGESIZE data read in configfs

On Tue, Oct 10, 2006 at 02:31:43PM -0700, Paul Menage wrote:
> >        NAK.  This forces a complex and inappropriate interface on the
> >majority of users, and doesn't honor configfs' simplicity-first design.
> 
> How is the seq_file interface complex and inappropriate? For the
> configfs clients it's basically a drop-in replacement for sprintf(),
> as Chandra's patches show.

	Well, they now have to learn seq_file.  They now get to assume
that "spewing large amounts of junk" is the default rather than "single
attribute", which is correct.  None of it is relevant for the majority
of correct users.
	It exposes the "I'm a file" knowledge down to the client module.
The entire point of configfs is that the "filesystem bits" are
independant of the "client bits".  To the client, it's an item
hierarchy.  To the user, the interface happens to be a filesystem.
	Technically, the seq_printf() as a drop-in replacement seems to
be functional.  I'm worried about lifetiming, but I think it's OK (what
do I mean?  If I open the file, I'd better not be able to remove the
client module until everything is torn down.  If I close the file, it
had better get all torn down before module_put() so that when
->release() returns, te module can safely be removed.  I *think* this
change satisfies these worries, but it's something that absolutely has
to be done right.  Yes, I'm very paranoid about this).
	My bigger worry is that we haven't solved the write side.  How
does one *set* a large attribute?  It had better not be multiple
attributes.  I know that your module doesn't set it, but hey, we don't
codify that requirement.  Perhaps a patch where we say "if you are a
large display attribute, we'll use seq_file and error on write because
it isn't allowed" but that leaves the old buffer-based approach for
normal-sized read-write attributes.

Joel

-- 

Life's Little Instruction Book #252

	"Take good care of those you love."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker@...cle.com
Phone: (650) 506-8127
-
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