On 28.11.2006 [10:41:13 +1100], David Gibson wrote:
> On Mon, Nov 27, 2006 at 12:21:05PM -0800, Nishanth Aravamudan wrote:
> > [resending due to user error on my end]
> >
> > On 20.11.2006 [10:45:49 -0800], Dave Hansen wrote:
> > > On Tue, 2006-11-14 at 09:55 -0800, Nishanth Aravamudan wrote:
> > > > + rm -rf `hugetlbfs_path`/elflink-uid-`id -u`
> > >
> > > Just a note: this will break with spaces in hugetlbfs_path. Even worse,
> > > if somebody was stupid, and did
> > >
> > > hugetlbfs_path="/ hugetlb_dir/"
> > >
> > > i.e.
> > >
> > > hugetlbfs_path=/\ hugetlb_dir/
> > >
> > > You'd 'rm -rf /'. Yikes.
> >
> > hugetlbfs_path is a function, defined as:
> >
> > function hugetlbfs_path() {
> > if [ -n "$HUGETLB_PATH" ]; then
> > echo "$HUGETLB_PATH"
> > else
> > grep hugetlbfs /proc/mounts | cut -f2 -d' '
> > fi
> > }
> >
> > First, if HUGETLB_PATH *is* set, all bets are off and that is
> > (albeit terrible) operator error... I'd be surprised, though, if the
> > tests would even get that far, since most everything would fail, in
> > very strange ways (not sure on that, not really willing to test on
> > this box, either though ;)
>
> Huh? Setting HUGETLB_PATH should work fine. It even won't have
> problems with spaces, as long as the double quotes are added around
> the call to hugetlbfs_path.
Yes, I meant without any changes. And I think, in general, if
HUGETLB_PATH is set and it's something crazy (and yes, I consider "/
hugetlb_dir/" to be crazy), then it's operator error. I think we should
do our best to avoid badness as a result, but it's still operator error.
> > So, that would require "/ hugetlbfs_dir/" to be the 3rd field in
> > /proc/mounts. I'm not positive, but I don't believe /proc/mounts is
> > quoted when printed, even if the mount point is quoted (do you know,
> > Dave?) -- which would mean we'd see the hugetlbfs_path as "/". Yes, that
> > would lead to issues, but I don't think it's a big deal. I suppose I
> > could modify my `rm -rf` to only delete the files *if* the test
> > succeeds? We may want to see them if they failed, after all...
>
> /proc/mounts certainly isn't quoted. Hat means the cut -d' ' will
> certainly break if the mountpoint has spaces in it. It's unclear how
> to fix this. Take as the path everything between the first and
> third-last spaces in the line, I guess (since I think all the other
> fields are guaranteed space-free). Yuck.
Yuck indeed.
> > In any case, how would you ever work around this in bash? We either
> > trust the user or we trust /proc/mounts to be sane. In both cases, if
> > they are not trustable, it's operator error (at the admin level,
> > presumably).
>
> It's not really a question of bash. bash can handle the spaces easily
> enough with the right quoting. But parsing /proc/mounts (or
> /etc/mtab) properly with spaces at all is tricky. I believe
> hugetlbfs_find_path() in hugeutils.c will also get this wrong.
>
> What I think we should do in the short term is to write a little
> utility program that invokes hugetlbfs_find_path() and use that to
> provide the path to the shell script. That won't actually make the
> parsing correct with spaces, but it does have some advantages:
> - The shell script is guaranteed to get the same path that the
> individual testcases do.
> - hugetlbfs_find_path() tests the path it finds, using
> statfs() to make sure it's really a hugetlbfs mount. That will limit
> the damage from a misparse, and at least prevent the nasty 'rm -rf /'
> problem.
A good point. Hrm, I've changed my patch for now to have the quoting as
suggested by Dave. I'm working on the linkshare problems mostly right
now, but once I'm done with that, I'll work on your idea for the small
helper application, as I think it's a good one.
Thanks,
Nish
--
Nishanth Aravamudan <[EMAIL PROTECTED]>
IBM Linux Technology Center
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Libhugetlbfs-devel mailing list
Libhugetlbfs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel
