Index: fs/binfmt_flat.c
===================================================================
--- fs/binfmt_flat.c (revision 6766)
+++ fs/binfmt_flat.c (working copy)
@@ -853,14 +853,23 @@ static int load_flat_shared_library(int
/* Open the file up */
bprm.filename = buf;
bprm.file = open_exec(bprm.filename);
+ bprm.cred = NULL;
res = PTR_ERR(bprm.file);
if (IS_ERR(bprm.file))
return res;
+ bprm.cred = prepare_exec_creds();
+ if (!bprm.cred)
+ goto out;
+
res = prepare_binprm(&bprm);
if (res <= (unsigned long)-4096)
res = load_flat_file(&bprm, libs, id, NULL, NULL);
+out:
+ if (bprm.cred)
+ abort_creds (bprm.cred);
+
if (bprm.file) {
allow_write_access(bprm.file);
fput(bprm.file);
On Mon, 22 Jun 2009, Bernd Schmidt wrote:
>
> This fixes a crash in 2.6.30 with shared flat binaries. prepare_binfmt now
> requires a cred pointer to be set up, which isn't done in binfmt_flat.c.
This has so many bugs in patch submission that I literally almost gave up
despite it being a trivial patch.
Attachements, wrong separators between explanation, wrong patch format
(hint: use "-p1" or some sane patch system that gets things right
_without_ crap), you name it. Very irritating.
I'm generally not interested in cleaning up after people who can't bother
to try to make patches in the right format. If you can't bother to spend
the small amount of time to make sure the patch is properly formatted, why
should I bother to apply it?
It literally looks like you used SVN to generate that sh*t-for-brains
patch. If you have to use svn, at least learn to make it generate proper
patches.
(How? I don't know. Maybe "svn diff -p1" works? Why would _anybody_ ever
use SVN for the kernel anyway? The "S" stands for "Substandard").
I applied this patch, but I want to just vent my frustration with it.
Linus
On Mon, 22 Jun 2009, Linus Torvalds wrote:
>
> I applied this patch, but I want to just vent my frustration with it.
Actually, I take that back. I applied it by hand, but then unapplied it,
because the patch also had
res = load_flat_file(&bprm, libs, id, NULL, NULL);
when my tree only has one "NULL", and always has had. So I'm not sure what
this is even based on.
Linus
From: Bernd Schmidt <[email protected]>
The new credentials code broke load_flat_shared_library() as it now uses
an uninitialized cred pointer.
Signed-off-by: Bernd Schmidt <[email protected]>
Signed-off-by: Mike Frysinger <[email protected]>
Acked-by: David Howells <[email protected]>
---
should be cleaned up to apply to the master branch
fs/binfmt_flat.c | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 697f6b5..33283cd 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -824,14 +824,23 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
/* Open the file up */
bprm.filename = buf;
bprm.file = open_exec(bprm.filename);
+ bprm.cred = NULL;
res = PTR_ERR(bprm.file);
if (IS_ERR(bprm.file))
return res;
+ bprm.cred = prepare_exec_creds();
+ if (!bprm.cred)
+ goto out;
+
res = prepare_binprm(&bprm);
if (res <= (unsigned long)-4096)
res = load_flat_file(&bprm, libs, id, NULL);
+ out:
+ if (bprm.cred)
+ abort_creds(bprm.cred);
+
if (bprm.file) {
allow_write_access(bprm.file);
fput(bprm.file);
--
1.6.3.1
>>>>> "Linus" == Linus Torvalds <[email protected]> writes:
Linus> On Mon, 22 Jun 2009, Bernd Schmidt wrote:
>>
>> This fixes a crash in 2.6.30 with shared flat binaries. prepare_binfmt now
>> requires a cred pointer to be set up, which isn't done in binfmt_flat.c.
Linus> This has so many bugs in patch submission that I literally
Linus> almost gave up despite it being a trivial patch.
I just recently submitted a patch, and it took me a while to figure
out a git flow for stupid, simple patches so that I, a moron, could
understand it. Here's what I've documented. Maybe it should be added
to Documentation/SubmittingPatches so that people like me, who only
rarely use git for anything, can use it to submit stuff properly?
###################################################################
# References
http://linux.yyz.us/git-howto.html
http://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html
http://www.kernel.org/pub/software/scm/git/docs/git-send-email.html
# One time commands
> apt-get install git-email
> git config --global user.name "John Stoffel"
> git config --global user.email "[email protected]"
# Only to be done infrequently
> mkdir /var/tmp/git-tree
> cd /var/tmp/git-tree
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git linux-2.6
# Make a new branch <branch> for the patch you're doing. In this
case, we'll do 'sym_hipd-ratelimit'
> git checkout -b sym_hipd-ratelimit
# Now edit the file
> vi drivers/scsi/sym53c8xx_2/sym_hipd.c
> git commit -a
# Put in a simple message of a line or two.
ratelimit parity error reporting, which helps alot on bootup over serial
console when you get 10,000+ lines of this message.
# Now exit the editor
# Check the commit, which is the most recent one by default
> git log -1
# See the actual patch with:
> git diff master..HEAD
# Make a patch you can send out, including any CC: and the signoff (-s)
> git format-patch [email protected] [email protected] -s
0001-ratelimit-parity-error-reporting-which-helps-alot-on.patch
# Now look the patch over and see if you need to edit the subject or
# anything
# In this case, I don't like the commit message
vi 0001-ratelimit-parity-error-reporting-which-helps-alot-on.patch
# Now do a dry run to send the email
> git send-email --dry-run [email protected] 0001-ratelimit-parity-error-reporting-which-helps-alot-on.patch
# Looks good, send for real
> git send-email [email protected] 0001-ratelimit-parity-error-reporting-which-helps-alot-on.patch
# Cleanup, got back to the main branch and merge in our change, just
# for fun
> git branch master
> git merge sym_hipd-ratelimit
On Mon, 22 Jun 2009, Mike Frysinger wrote:
>
> should be cleaned up to apply to the master branch
Hmm. Can somebody explain why we even bother to test bprm.cred for NULL
that second time?
Or why we test bprm.file, for that matter? How could it possibly suddenly
become NULL?
Finally, if that bprm.cred _is_ NULL in the first test, then 'res' will be
NULL (from the previous statement), and with the "goto out" we'll return
_success_ from this function when we failed to allocate a cred.
IOW, the whole patch really seems to be total and utter crap. Why didn't
people spend a bit more effort lookin at it?
IOW, shouldn't the patch be something like the appended?
UNTESTED. I did not compile this, or the previous patch. I have not tried
it. I'm not going to commit it. I'm hoping to get a patch back that is
tested and/or explains my concerns with the previous one..
Linus
---
fs/binfmt_flat.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 697f6b5..e92f229 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -828,15 +828,22 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
if (IS_ERR(bprm.file))
return res;
+ bprm.cred = prepare_exec_creds();
+ res = -ENOMEM;
+ if (!bprm.cred)
+ goto out;
+
res = prepare_binprm(&bprm);
if (res <= (unsigned long)-4096)
res = load_flat_file(&bprm, libs, id, NULL);
- if (bprm.file) {
- allow_write_access(bprm.file);
- fput(bprm.file);
- bprm.file = NULL;
- }
+
+ abort_creds(bprm.cred);
+
+out:
+ allow_write_access(bprm.file);
+ fput(bprm.file);
+
return(res);
}
On Mon, Jun 22, 2009 at 16:48, Linus Torvalds wrote:
> On Mon, 22 Jun 2009, Mike Frysinger wrote:
>> should be cleaned up to apply to the master branch
>
> Hmm. Can somebody explain why we even bother to test bprm.cred for NULL
> that second time?
Bernd and/or David will have to cover this one
the previous discussion was here, but i dont think it answers the
questions you've raised:
http://thread.gmane.org/gmane.linux.hardware.blackfin.kernel.devel/1905
-mike
On Mon, 22 Jun 2009, John Stoffel wrote:
>
> I just recently submitted a patch, and it took me a while to figure
> out a git flow for stupid, simple patches so that I, a moron, could
> understand it.
Well, your documented flow is pretty advanced, actually. In fact, much
more advanced than what _I_ often do when I send out patches.
So I'm not saying that your flow is wrong at all, but for "casual"
patches, there are simplifications..
> # One time commands
> > apt-get install git-email
> > git config --global user.name "John Stoffel"
> > git config --global user.email "[email protected]"
git-email can be useful, but it's really only useful if you plan on
sending lots of patches. A lot of casual users may fin it too much
overhead).
The user name/email is always good to set up with git, though.
Of course, the really casual use won't ever even care, because it won't
necessarily even commit things! But setting it up (once) certainly won't
hurt.
> # Only to be done infrequently
> > mkdir /var/tmp/git-tree
> > cd /var/tmp/git-tree
> > git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git linux-2.6
Yes. Something like this, although "/var/tmp" is an odd place to put it
unless it's really a one-off thing.
> # Make a new branch <branch> for the patch you're doing. In this
> case, we'll do 'sym_hipd-ratelimit'
Ok, this is where your casual use gets to be already pretty advanced.
I would suggest starting out using the kernel git repository as more of a
"anonymous cvs" replacement, ie to just track upstream. So just keep doing
git pull origin
every once in a while, which is a really convenient way to just say "get
the latest version and update the working tree"
NOTE! It's really convenient _only_ if you don't actually do commits. If
you do your own work, it's going to do a merge etc, and that's part of the
"advanced" feature. The above is all assuming that you do NOT commit, and
you really consider it a read-only tree with perhaps some random small
local changes on top for testing.
If you are going to do anything fancier, then your "create a branch for
testing" is good. But for somebody who doesn't expect to really be a
kernel developer, starting out with a usage model based on anoncvs is
probably a more natural one.
Then, just edit the files, and use "git diff". That, along with "git pull"
would be the _only_ command you'd ever use - and you now have a nice
anoncvs-like environment to get used to git with, without having to
actually know a whole lot.
Now, what happens if your modifications touch a file that "git pull" will
update? In that case, "git pull" will will just refuse to update the file,
and do nothing. You can decide at that point to do one of several
things:
- maybe you just want to just stay at that base (not pull updates for a
while, until you're all done with your changes)
- decide to continue with the "anoncvs" workflow, but try to just
forward-port the patches: learn the "git stash" command, and do
git stash # save your changes
git pull # do the update
git stash apply # re-apply your changes
which obviously can be complicated if you have real conflicts, but no
worse than the alternatives. If you change code that changed upstream,
you'll have to resolve the thing _somehow_, and the above won't be that
different from what you're used to with "cvs update" or whatever.
- decide that you are actually going to learn to use git branches etc,
and stop using git as just a smarter 'anoncvs' thing. This is when you
want to learn about commit, branches, and perhaps "git pull --rebase"
and friends.
In other words, you really don't have to jump in with both feet. You could
just dip your toe in the git waters before you do anything fancier.
Linus
Linus> On Mon, 22 Jun 2009, John Stoffel wrote:
>>
>> I just recently submitted a patch, and it took me a while to figure
>> out a git flow for stupid, simple patches so that I, a moron, could
>> understand it.
Linus> Well, your documented flow is pretty advanced, actually. In
Linus> fact, much more advanced than what _I_ often do when I send out
Linus> patches.
Heh, but I can't write patches in my sleep like you can I bet. And I
was looking for an easy to use method that would let me keep a small
subset of patches I write or test easily accesible. And since I'm NOT
a C coder by day, but a SysAdmin who hacks Perl mostly... this seems
to fit into the spirit of Git usage in the kernel.
Linus> So I'm not saying that your flow is wrong at all, but for "casual"
Linus> patches, there are simplifications..
Excellent! I figured I was missing stuff, but like I said...
>> # One time commands
>> > apt-get install git-email
>> > git config --global user.name "John Stoffel"
>> > git config --global user.email "[email protected]"
Linus> git-email can be useful, but it's really only useful if you
Linus> plan on sending lots of patches. A lot of casual users may fin
Linus> it too much overhead).
Linus> The user name/email is always good to set up with git, though.
Linus> Of course, the really casual use won't ever even care, because
Linus> it won't necessarily even commit things! But setting it up
Linus> (once) certainly won't hurt.
I think git-email is actually really useful, even for occasional
users, since it makes sure you do it right and send a good patch,
without whitespace damage, proper signed-off-by lines, etc. Much
easier than crafting an email by hand.
Esp when I wanted to make it as easy as possible on you and James to
actually accept and push my patch upsteam. Keeping it easy on the
sub-system maintainers is a key thing in my mind.
>> # Only to be done infrequently
>> > mkdir /var/tmp/git-tree
>> > cd /var/tmp/git-tree
>> > git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git linux-2.6
Linus> Yes. Something like this, although "/var/tmp" is an odd place
Linus> to put it unless it's really a one-off thing.
Me being lazy and knowing that /var/tmp doesn't get nuked on reboots.
For real documentation, I'd clean it up into another path.
>> # Make a new branch <branch> for the patch you're doing. In this
>> case, we'll do 'sym_hipd-ratelimit'
Linus> Ok, this is where your casual use gets to be already pretty
Linus> advanced.
I figured that if I put my patch into a branch, I could then keep
pulling your tree into mine, and then merging my patch upsteam as
needed if it doesn't make it in this time.
Linus> I would suggest starting out using the kernel git repository as
Linus> more of a "anonymous cvs" replacement, ie to just track
Linus> upstream. So just keep doing
I've *never* done any development using CVS, etc. Just some basic
version control scripted to manage files for sysadmin type work. So
version control is just a pain for me to do, since I do it so
infrequently.
Linus> git pull origin
Linus> every once in a while, which is a really convenient way to just
Linus> say "get the latest version and update the working tree"
git pull
will work too, assuming I'm on the master branch, right?
Linus> NOTE! It's really convenient _only_ if you don't actually do
Linus> commits. If you do your own work, it's going to do a merge etc,
Linus> and that's part of the "advanced" feature. The above is all
Linus> assuming that you do NOT commit, and you really consider it a
Linus> read-only tree with perhaps some random small local changes on
Linus> top for testing.
Should I still do branches, even if I *don't* do a commit on those
various branches?
Linus> If you are going to do anything fancier, then your "create a
Linus> branch for testing" is good. But for somebody who doesn't
Linus> expect to really be a kernel developer, starting out with a
Linus> usage model based on anoncvs is probably a more natural one.
Heh, just having some notes which let's me hack in a patch and keep
track of where I am and juggle branches a bit is a *huge* help.
Linus> Then, just edit the files, and use "git diff". That, along with
Linus> "git pull" would be the _only_ command you'd ever use - and you
Linus> now have a nice anoncvs-like environment to get used to git
Linus> with, without having to actually know a whole lot.
So say I want to manage multiple branches, since I want to test some
patches out, or various kernel revisions, or even git bisection?
Or should I just use a completely seperate tree for a bisection run?
Linus> Now, what happens if your modifications touch a file that "git
Linus> pull" will update? In that case, "git pull" will will just
Linus> refuse to update the file, and do nothing. You can decide at
Linus> that point to do one of several things:
Linus> - maybe you just want to just stay at that base (not pull
Linus> updates for a while, until you're all done with your
Linus> changes)
Linus> - decide to continue with the "anoncvs" workflow, but try to
Linus> just forward-port the patches: learn the "git stash"
Linus> command, and do
Linus> git stash # save your changes
Linus> git pull # do the update
Linus> git stash apply # re-apply your changes
Ohh! This is a new one to me. Gotta remember this one, that's huge!
Maybe what I've written should be updated to use this method instead?
Linus> which obviously can be complicated if you have real
Linus> conflicts, but no worse than the alternatives. If you change
Linus> code that changed upstream, you'll have to resolve the thing
Linus> _somehow_, and the above won't be that different from what
Linus> you're used to with "cvs update" or whatever.
Linus> - decide that you are actually going to learn to use git
Linus> branches etc, and stop using git as just a smarter 'anoncvs'
Linus> thing. This is when you want to learn about commit,
Linus> branches, and perhaps "git pull --rebase" and friends.
Heh, from all I've seen with your rants, git pull --rebase isn't what
I want to do if I can help it. :] Even though I'll probably never
publish a branch, just small patches occasionally at times.
Linus> In other words, you really don't have to jump in with both
Linus> feet. You could just dip your toe in the git waters before you
Linus> do anything fancier.
I prefer to think that I've just waded into the kiddy pool with
hipboots on, before I try to do any *real* fly (bug) fishing.
Thanks for the reply, as usual, I've learned something new. Now to
put it into practice.
John
On Mon, 22 Jun 2009, John Stoffel wrote:
>
> Linus> Then, just edit the files, and use "git diff". That, along with
> Linus> "git pull" would be the _only_ command you'd ever use - and you
> Linus> now have a nice anoncvs-like environment to get used to git
> Linus> with, without having to actually know a whole lot.
>
> So say I want to manage multiple branches, since I want to test some
> patches out, or various kernel revisions, or even git bisection?
>
> Or should I just use a completely seperate tree for a bisection run?
Oh, it really does sound like you want to do the whole "multiple branches"
thing, and want to do way more with git than just use it as an anoncvs
replacement.
So your workflow is fine.
My clarification was really meant for the case where people were perhaps
more used to the CVS kind of model (and SVN is very much the same one).
I bet there's lots of people who are used to anoncvs and carrying their
patches (uncommitted) in their trees, and then doing "cvs update".
So I tried to just outline the git version of that.
It's a much weaker model, and not at all as capable, but it _is_ simpler.
> Linus> git stash # save your changes
> Linus> git pull # do the update
> Linus> git stash apply # re-apply your changes
>
> Ohh! This is a new one to me. Gotta remember this one, that's huge!
> Maybe what I've written should be updated to use this method instead?
Your multiple branch approach is much better since you do commits etc. A
commit is a long-term stash, and you can (obviously) queue them on top of
each other.
With "git stash", you can have multiple different stashed things too, but
they don't queue up on each other - they are just random independent
patches that you've stashed away because they were inconvenient at some
point.
So the real "branches + commits" model is what a real git user does, and
your description wasn't bad at all. It was just perhaps more of a stretch
than some people who are used to CVS would necessarily want to begin with.
> Heh, from all I've seen with your rants, git pull --rebase isn't what
> I want to do if I can help it. :] Even though I'll probably never
> publish a branch, just small patches occasionally at times.
There's nothing wrong with "git pull --rebase" if you're a "leaf
developer", ie you don't publish your git tree, and you don't integrate
work from other peoples git trees.
Doing "git pull --rebase" (or "git fetch" + "git rebase origin", which is
the same thing) is very convenient for people who do commit their changes
with git, and aren't ready to publish them.
I actually do it all the time myself - in my own private git source tree.
I send git patches to Junio by email, and I don't publish my own git tree
any more, since Junio is the real maintainer, and has been doing such a
stellar job.
So when it comes to git, I occasionally have some git work going on
myself, and I tend to maintain those as commits in my private tree that I
then update with "git pull --rebase". It's an excellent _private_
workflow, because it means that I can trivially keep my own work
up-to-date, and if part of it is something that I sent to Junio for
merging, if/when he merges it, when I do "git pull --rebase" my own
private commit automatically goes away since it's no longer needed, and
got merged.
So it's true that I rant against people doing rebase, but I rant against
maintainers who either rebase other peoples commits (it's happily getting
fairly rare), or who keep a private tree that they rebase all the time,
and then they publish what is obviously _totally_ untested crap, because I
can tell that they rebased within the last few hours, and thus the weeks
or months of work that they then publish has been essentially made public
with zero testing.
So rebase has huge downsides, but for private git patch queues it's
wonderful. There's a reason 'git rebase' is very much part of the core git
distribution, it's not some kind of second-class citizen.
Linus
Linus Torvalds wrote:
> IOW, the whole patch really seems to be total and utter crap. Why didn't
> people spend a bit more effort lookin at it?
Just sloppiness, I'm afraid.
> IOW, shouldn't the patch be something like the appended?
>
> UNTESTED. I did not compile this, or the previous patch. I have not tried
> it. I'm not going to commit it. I'm hoping to get a patch back that is
> tested and/or explains my concerns with the previous one..
Your version seems to work fine. Please apply it.
Bernd
--
This footer brought to you by insane German lawmakers.
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
"John Stoffel" <[email protected]> writes:
>> git commit -a
>
> # Put in a simple message of a line or two.
>
> ratelimit parity error reporting, which helps alot on bootup over serial
> console when you get 10,000+ lines of this message.
Doesn't this violate the accepted standard format for commit messages in
the linux kernel project?
Perhaps Documentation/SubmittingPatches #2 and #15 need to be read, or
clarified, or both.
On Tue, Jun 23, 2009 at 11:14:02AM +0100, Bernd Schmidt wrote:
> Linus Torvalds wrote:
> > IOW, the whole patch really seems to be total and utter crap. Why didn't
> > people spend a bit more effort lookin at it?
>
> Just sloppiness, I'm afraid.
>
> > IOW, shouldn't the patch be something like the appended?
> >
> > UNTESTED. I did not compile this, or the previous patch. I have not tried
> > it. I'm not going to commit it. I'm hoping to get a patch back that is
> > tested and/or explains my concerns with the previous one..
>
> Your version seems to work fine. Please apply it.
Linus, this doesn't seem to be in your tree yet. Did it get lost in the
noise?
thanks,
greg k-h