Re: Regression - Xorg start failed
* Dave Airlie (airl...@gmail.com) wrote: > Yes most likely, I'm not at the machine now but I'll see if I can the > AVC off it later. Great, thanks. > On Fedora 13? > > F14/rawhide seem fine. I'm on F14. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Regression - Xorg start failed
* Dave Airlie (airl...@gmail.com) wrote: > On Sun, Feb 13, 2011 at 4:22 PM, Dave Young wrote: > > With kernel built from current linus's tree, I can not start xorg, > > it failed with: > > Me too!, > > Thanks for bisceting this, I just tried -rc4 on my Fedora 13 laptop > and wasn't looking forward to bisecting it. > > If this isn't hitting later distros its probably an updated > libpciaccess that fixed something. > > libpciaccess-0.10.9-2.20091209.fc13.i686 is what is on this box. > > Probably should revert first, then work out what is crapping out libpciaccess. Are you running with SELinux enabled, if so can you paste the AVC error? It's working for me here. thanks, -chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Regression - Xorg start failed
* Linus Torvalds (torva...@linux-foundation.org) wrote:
> On Sat, Feb 12, 2011 at 11:53 PM, Dave Airlie wrote:
> > Probably should revert first, then work out what is crapping out
> > libpciaccess.
>
> Yeah, I'll revert. The patch is one of those "obviously a good idea,
> but in practice it's not something we can change now".
Turns out I'm just a bona fide idiot.
I was not testing the right kernel _and_ didn't get the logic right.
sorry for the screw up,
-chris
---
Subject: [PATCH] pci: use security_capable correctly during config space read
Commit 47970b1 ("pci: use security_capable() when checking capablities
during config space read") is just plain broken. The normal capable()
interface returns true on success, but the LSM interface returns 0 on
success.
Signed-off-by: Chris Wright
---
I've tested this quickly (lspci behaviour is as expected).
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index f7771f3..ea25e5b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -369,7 +369,7 @@ pci_read_config(struct file *filp, struct kobject *kobj,
u8 *data = (u8*) buf;
/* Several chips lock up trying to read undefined config space */
- if (security_capable(filp->f_cred, CAP_SYS_ADMIN)) {
+ if (security_capable(filp->f_cred, CAP_SYS_ADMIN) == 0) {
size = dev->cfg_size;
} else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
size = 128;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Regression - Xorg start failed
* Sedat Dilek (sedat.di...@googlemail.com) wrote: > [ As I am not subscribed to both MLs and lazy to pick up all involved people ] > > Hi Chris, > > The original patch had also this part (see the revert in [1]): > ... > +#include > ... > > Your new patch in [2] is missing it or is it not required? It was just a patch on top of the bad commit (47970b1b), the #include is required. thanks, -chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Regression - Xorg start failed
* Dave Young (hidave.darks...@gmail.com) wrote:
> On Sun, Feb 13, 2011 at 04:35:31PM -0800, Chris Wright wrote:
> > Commit 47970b1 ("pci: use security_capable() when checking capablities
> > during config space read") is just plain broken. The normal capable()
> > interface returns true on success, but the LSM interface returns 0 on
> > success.
>
> Chris, linus has reverted the original commit, so this does not apply.
Right, I'll send a new patch to James.
> Anyway I have tested this one, it works well. Feel free add my Tested-by line.
Thanks for confirming the fix Dave.
thanks,
-chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Regression - Xorg start failed
* Sedat Dilek (sedat.di...@googlemail.com) wrote: > On Mon, Feb 14, 2011 at 5:20 PM, Chris Wright wrote: > > * Sedat Dilek (sedat.di...@googlemail.com) wrote: > >> [ As I am not subscribed to both MLs and lazy to pick up all involved > >> people ] > >> > >> Hi Chris, > >> > >> The original patch had also this part (see the revert in [1]): > >> ... > >> +#include > >> ... > >> > >> Your new patch in [2] is missing it or is it not required? > > > > It was just a patch on top of the bad commit (47970b1b), the #include is > > required. > > > > thanks, > > -chris > > > > OK, now I see. > Might be good to push a v2 as the initial patch was reverted in the meantime? > The explanations of the fixup patch were helpful (to me) and should be > included. Yeah, I'll send a new patch to James. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] pci: use security_capable() when checking capablities during config space read
This reintroduces commit 47970b1b which was subsequently reverted
as f00eaeea. The original change was broken and caused X startup
failures and generally made privileged processes incapable of reading
device dependent config space. The normal capable() interface returns
true on success, but the LSM interface returns 0 on success. This thinko
is now fixed in this patch, and has been confirmed to work properly.
So, once again...Eric Paris noted that commit de139a3 ("pci: check caps
from sysfs file open to read device dependent config space") caused the
capability check to bypass security modules and potentially auditing.
Rectify this by calling security_capable() when checking the open file's
capabilities for config space reads.
Reported-by: Eric Paris
Tested-by: Dave Young
Acked-by: James Morris
Cc: Dave Airlie
Cc: Alex Riesen
Cc: Sedat Dilek
Cc: Linus Torvalds
Signed-off-by: Chris Wright
---
v2: added Reported-by Eric
v3: fix logic screw up
drivers/pci/pci-sysfs.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 8ecaac9..ea25e5b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include "pci.h"
@@ -368,7 +369,7 @@ pci_read_config(struct file *filp, struct kobject *kobj,
u8 *data = (u8*) buf;
/* Several chips lock up trying to read undefined config space */
- if (cap_raised(filp->f_cred->cap_effective, CAP_SYS_ADMIN)) {
+ if (security_capable(filp->f_cred, CAP_SYS_ADMIN) == 0) {
size = dev->cfg_size;
} else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
size = 128;
--
1.7.3.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Regression - Xorg start failed
* Dave Airlie (airlied at gmail.com) wrote: > Yes most likely, I'm not at the machine now but I'll see if I can the > AVC off it later. Great, thanks. > On Fedora 13? > > F14/rawhide seem fine. I'm on F14.
Regression - Xorg start failed
* Dave Airlie (airlied at gmail.com) wrote: > On Sun, Feb 13, 2011 at 4:22 PM, Dave Young > wrote: > > With kernel built from current linus's tree, I can not start xorg, > > it failed with: > > Me too!, > > Thanks for bisceting this, I just tried -rc4 on my Fedora 13 laptop > and wasn't looking forward to bisecting it. > > If this isn't hitting later distros its probably an updated > libpciaccess that fixed something. > > libpciaccess-0.10.9-2.20091209.fc13.i686 is what is on this box. > > Probably should revert first, then work out what is crapping out libpciaccess. Are you running with SELinux enabled, if so can you paste the AVC error? It's working for me here. thanks, -chris
Regression - Xorg start failed
* Linus Torvalds (torvalds at linux-foundation.org) wrote:
> On Sat, Feb 12, 2011 at 11:53 PM, Dave Airlie wrote:
> > Probably should revert first, then work out what is crapping out
> > libpciaccess.
>
> Yeah, I'll revert. The patch is one of those "obviously a good idea,
> but in practice it's not something we can change now".
Turns out I'm just a bona fide idiot.
I was not testing the right kernel _and_ didn't get the logic right.
sorry for the screw up,
-chris
---
Subject: [PATCH] pci: use security_capable correctly during config space read
Commit 47970b1 ("pci: use security_capable() when checking capablities
during config space read") is just plain broken. The normal capable()
interface returns true on success, but the LSM interface returns 0 on
success.
Signed-off-by: Chris Wright
---
I've tested this quickly (lspci behaviour is as expected).
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index f7771f3..ea25e5b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -369,7 +369,7 @@ pci_read_config(struct file *filp, struct kobject *kobj,
u8 *data = (u8*) buf;
/* Several chips lock up trying to read undefined config space */
- if (security_capable(filp->f_cred, CAP_SYS_ADMIN)) {
+ if (security_capable(filp->f_cred, CAP_SYS_ADMIN) == 0) {
size = dev->cfg_size;
} else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
size = 128;
Regression - Xorg start failed
* Sedat Dilek (sedat.dilek at googlemail.com) wrote: > [ As I am not subscribed to both MLs and lazy to pick up all involved people ] > > Hi Chris, > > The original patch had also this part (see the revert in [1]): > ... > +#include > ... > > Your new patch in [2] is missing it or is it not required? It was just a patch on top of the bad commit (47970b1b), the #include is required. thanks, -chris
Regression - Xorg start failed
* Dave Young (hidave.darkstar at gmail.com) wrote:
> On Sun, Feb 13, 2011 at 04:35:31PM -0800, Chris Wright wrote:
> > Commit 47970b1 ("pci: use security_capable() when checking capablities
> > during config space read") is just plain broken. The normal capable()
> > interface returns true on success, but the LSM interface returns 0 on
> > success.
>
> Chris, linus has reverted the original commit, so this does not apply.
Right, I'll send a new patch to James.
> Anyway I have tested this one, it works well. Feel free add my Tested-by line.
Thanks for confirming the fix Dave.
thanks,
-chris
Regression - Xorg start failed
* Sedat Dilek (sedat.dilek at googlemail.com) wrote: > On Mon, Feb 14, 2011 at 5:20 PM, Chris Wright wrote: > > * Sedat Dilek (sedat.dilek at googlemail.com) wrote: > >> [ As I am not subscribed to both MLs and lazy to pick up all involved > >> people ] > >> > >> Hi Chris, > >> > >> The original patch had also this part (see the revert in [1]): > >> ... > >> +#include > >> ... > >> > >> Your new patch in [2] is missing it or is it not required? > > > > It was just a patch on top of the bad commit (47970b1b), the #include is > > required. > > > > thanks, > > -chris > > > > OK, now I see. > Might be good to push a v2 as the initial patch was reverted in the meantime? > The explanations of the fixup patch were helpful (to me) and should be > included. Yeah, I'll send a new patch to James.
[PATCH v3] pci: use security_capable() when checking capablities during config space read
This reintroduces commit 47970b1b which was subsequently reverted
as f00eaeea. The original change was broken and caused X startup
failures and generally made privileged processes incapable of reading
device dependent config space. The normal capable() interface returns
true on success, but the LSM interface returns 0 on success. This thinko
is now fixed in this patch, and has been confirmed to work properly.
So, once again...Eric Paris noted that commit de139a3 ("pci: check caps
from sysfs file open to read device dependent config space") caused the
capability check to bypass security modules and potentially auditing.
Rectify this by calling security_capable() when checking the open file's
capabilities for config space reads.
Reported-by: Eric Paris
Tested-by: Dave Young
Acked-by: James Morris
Cc: Dave Airlie
Cc: Alex Riesen
Cc: Sedat Dilek
Cc: Linus Torvalds
Signed-off-by: Chris Wright
---
v2: added Reported-by Eric
v3: fix logic screw up
drivers/pci/pci-sysfs.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 8ecaac9..ea25e5b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include "pci.h"
@@ -368,7 +369,7 @@ pci_read_config(struct file *filp, struct kobject *kobj,
u8 *data = (u8*) buf;
/* Several chips lock up trying to read undefined config space */
- if (cap_raised(filp->f_cred->cap_effective, CAP_SYS_ADMIN)) {
+ if (security_capable(filp->f_cred, CAP_SYS_ADMIN) == 0) {
size = dev->cfg_size;
} else if (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS) {
size = 128;
--
1.7.3.4
