Re: [PATCH 0/3] Add O_DENY* flags to fcntl and cifs
On Mon, 10 Dec 2012 11:41:16 -0500
"J. Bruce Fields" wrote:
> On Sat, Dec 08, 2012 at 12:43:14AM +0400, Pavel Shilovsky wrote:
> > The problem is the possibility of denial-of-service attacks here. We
> > can try to prevent them by:
> > 1) specifying an extra security bit on the file that indicates that
> > share flags are accepted (like we have for mandatory locks now) and
> > setting it for neccessary files only, or
> > 2) adding a special mount option (but it it probably makes sense if
> > we decided to add this support for CIFS and NFS only).
>
> In the case of knfsd and samba exporting a common filesystem, you'd also
> want to be able to enforce it on the exported filesystem.
>
Sorry for chiming in late on this, but I've been looking at this
problem from the other end as well, from the POV of a fileserver. For
there, you absolutely do want to have some mechanism for enforcing this
on local filesystems.
Currently, file servers generally enforce share reservations
internally. The upshot is that they're not aware when other tasks
outside the server modify a file. This is also problematic too in many
common fileserving situations -- when exporting files via both NFS and
SMB, for instance.
One thing that's important to note is that there is already some
support for this in the kernel. The LOCK_MAND flag and its associates
are intended for just this purpose. Samba even already calls into the
kernel to set LOCK_MAND locks, but the kernel just passes them through
to the fs. Rumor has it that GPFS does something with these flags, but
I can't confirm that.
Of course, LOCK_MAND is racy since you can't set it on open(), but it
might be nice to use that as a starting point for trying to add this
support.
At the very least, if we're going to do this, we need to consider what
to do with the LOCK_MAND interfaces. As a starting point for
discussion, here's a patch that I was playing with a few months ago. I
haven't had much time to really spend on this project, but it may be
worthwhile to consider. It works, but I'm not sure about the
semantics...
-[snip]
locks: add enforcement of LOCK_MAND locks
The LOCK_MAND lock mechanism is currently a no-op for any in-tree
filesystem. The flags are passed to f_ops->flock, but the standard
flock routines basically ignore them.
Change this by adding enforcement against other LOCK_MAND locks. Also,
assume that LOCK_MAND also implies LOCK_NB.
Signed-off-by: Jeff Layton
---
fs/locks.c | 45 ++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index 814c51d..736e38b 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -625,6 +625,43 @@ static int posix_locks_conflict(struct file_lock
*caller_fl, struct file_lock *s
return (locks_conflict(caller_fl, sys_fl));
}
+/*
+ * locks_mand_conflict - Determine if there's a share reservation conflict
+ * @caller_fl: lock we're attempting to acquire
+ * @sys_fl: lock already present on system that we're checking against
+ *
+ * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE
+ * tell us whether the reservation allows other readers and writers.
+ *
+ * We only check against other LOCK_MAND locks, so applications that want to
+ * use share mode locking will only conflict against one another. "normal"
+ * applications that open files won't be affected by and won't themselves
+ * affect the share reservations.
+ */
+static int locks_mand_conflict(struct file_lock *caller_fl,
+ struct file_lock *sys_fl)
+{
+ unsigned char caller_type = caller_fl->fl_type;
+ unsigned char sys_type = sys_fl->fl_type;
+ fmode_t caller_fmode = caller_fl->fl_file->f_mode;
+ fmode_t sys_fmode = sys_fl->fl_file->f_mode;
+
+ /* they can only conflict if they're both LOCK_MAND */
+ if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND))
+ return 0;
+
+ if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ))
+ return 1;
+ if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE))
+ return 1;
+ if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ))
+ return 1;
+ if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE))
+ return 1;
+
+ return 0;
+}
+
/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
* checking before calling the locks_conflict().
*/
@@ -633,9 +670,11 @@ static int flock_locks_conflict(struct file_lock
*caller_fl, struct file_lock *s
/* FLOCK locks referring to the same filp do not conflict with
* each other.
*/
- if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file))
- return (0);
+ if (!IS_FLOCK(sys_fl))
+ return 0;
if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND))
+
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, Dec 11, 2012 at 12:37 PM, Hans Leidekker wrote: > On Tue, 2012-12-11 at 11:52 -0800, Juan Lang wrote: > > On Tue, Dec 11, 2012 at 6:10 AM, Hans Leidekker > wrote: > > On Tue, 2012-12-11 at 14:52 +0100, Jacek Caban wrote: > > > On 12/11/12 09:45, Hans Leidekker wrote: > > > > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a > test which shows that > > > > revocation checks fail for the certificate on outlook.comwhen > > passed straight > > > > to CertVerifyRevocation. The reason is that a CRL link > specified in the > > > > certificate does not resolve. > > > > > > > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a > test which makes > > > > a secure connection to outlook.com from wininet and shows > that this succeeds. > > > > > > > > My conclusion is that native wininet doesn't perform > revocation checks. > > > > > > Your tests prove that we should relax our verification on > > > CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To > prove that > > > revocation checks are not made, a test with truly revoked cert > would be > > > needed. > > > > > > True, though to perform the revocation check the CRL has to be > retrieved and my > > tests with wireshark didn't show any signs of that. > > > > > > Would adding to the tests as part of this patch be a bad thing? > > I thought about that but I am hesitant to use a random site that's not > under our > control. > > The alternative is to setup our own site with a certificate that only > fails the > revocation check, which I think means that we need to have it signed by a > trusted > root and then revoked. I'm not sure we have the means to do that currently. > > Do you have any suggestions? > I agree with you that this is a little finicky to test, but I think it's tractable. The desired outcome is a server trusted by the client under test, presenting a valid server cert, under two conditions: where the server cert's revocation can't be tested, and where the server's cert is actually revoked. First the easy bit: testing what happens when the revocation can't be checked involves a valid cert with a CRL distribution point that doesn't respond. Easy, as long as you can create the server cert that the client trusts. Getting the client to trust the server cert can be as easy as ignoring untrusted root errors, if you don't think this impacts the revocation results. Returning revocation is straightforward enough, assuming you have a server under your control. Now, the server: is it possible to have another foo.winehq.org provisioned that returns an arbitrary cert? If not, it's possible to run a server locally, but this is a lot more code overhead, so probably not worth it for this go around. --Juan
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, 2012-12-11 at 11:52 -0800, Juan Lang wrote: > On Tue, Dec 11, 2012 at 6:10 AM, Hans Leidekker wrote: > On Tue, 2012-12-11 at 14:52 +0100, Jacek Caban wrote: > > On 12/11/12 09:45, Hans Leidekker wrote: > > > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test > which shows that > > > revocation checks fail for the certificate on outlook.com when > passed straight > > > to CertVerifyRevocation. The reason is that a CRL link specified > in the > > > certificate does not resolve. > > > > > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test > which makes > > > a secure connection to outlook.com from wininet and shows that > this succeeds. > > > > > > My conclusion is that native wininet doesn't perform revocation > checks. > > > > Your tests prove that we should relax our verification on > > CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that > > revocation checks are not made, a test with truly revoked cert > would be > > needed. > > > True, though to perform the revocation check the CRL has to be > retrieved and my > tests with wireshark didn't show any signs of that. > > > Would adding to the tests as part of this patch be a bad thing? I thought about that but I am hesitant to use a random site that's not under our control. The alternative is to setup our own site with a certificate that only fails the revocation check, which I think means that we need to have it signed by a trusted root and then revoked. I'm not sure we have the means to do that currently. Do you have any suggestions?
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, Dec 11, 2012 at 6:10 AM, Hans Leidekker wrote: > On Tue, 2012-12-11 at 14:52 +0100, Jacek Caban wrote: > > On 12/11/12 09:45, Hans Leidekker wrote: > > > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test which > shows that > > > revocation checks fail for the certificate on outlook.com when passed > straight > > > to CertVerifyRevocation. The reason is that a CRL link specified in the > > > certificate does not resolve. > > > > > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test which > makes > > > a secure connection to outlook.com from wininet and shows that this > succeeds. > > > > > > My conclusion is that native wininet doesn't perform revocation checks. > > > > Your tests prove that we should relax our verification on > > CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that > > revocation checks are not made, a test with truly revoked cert would be > > needed. > > True, though to perform the revocation check the CRL has to be retrieved > and my > tests with wireshark didn't show any signs of that. > Would adding to the tests as part of this patch be a bad thing? --Juan
Re: RFC: Remove auto-scan of ALSA devices from winealsa.drv
On Tue, Dec 11, 2012 at 9:39 AM, Max TenEyck Woodbury wrote: > On 12/11/2012 10:46 AM, Henri Verbeet wrote: >> >> On 11 December 2012 16:05, wrote: >>> >>> Cost to users: >>> >>> Users with a working ALSA device "default" should experience no >>> drawback, only benefits. I believe this is the vast majority of users. >>> >>> Users that edit their ~/.asoundrc to define other devices without >>> simultaneously overriding !default will have to additionally edit the >>> Wine registry to name their working ALSA capture and render devices, >>> e.g. "asnoop" and "amix". >>> >> It will also pretty much just remove device selection on setup with >> multiple audio devices, which is actually fairly common these days >> with USB audio devices and HDMI outputs on graphics cards. I think the >> correct approach would to work with upstream ALSA to fix things, >> instead of just removing device enumeration. >> >> > I do not think this is a particularly good idea. I do have two sound > systems on my machine and I have assigned each to different roles. That > seems to work quite well. What does not work well is leaving the role > set to 'default'. That results in the selection of the highest latency > device with corresponding stutters and over-runs. The current > requirement for selecting an output device is mildly annoying, but no > where near as annoying as being forced to use a faulty device. You'd still be able to select a different device in the registry. -- -Austin
Re: RFC: Remove auto-scan of ALSA devices from winealsa.drv
On 12/11/2012 10:46 AM, Henri Verbeet wrote: On 11 December 2012 16:05, wrote: Cost to users: Users with a working ALSA device "default" should experience no drawback, only benefits. I believe this is the vast majority of users. Users that edit their ~/.asoundrc to define other devices without simultaneously overriding !default will have to additionally edit the Wine registry to name their working ALSA capture and render devices, e.g. "asnoop" and "amix". It will also pretty much just remove device selection on setup with multiple audio devices, which is actually fairly common these days with USB audio devices and HDMI outputs on graphics cards. I think the correct approach would to work with upstream ALSA to fix things, instead of just removing device enumeration. I do not think this is a particularly good idea. I do have two sound systems on my machine and I have assigned each to different roles. That seems to work quite well. What does not work well is leaving the role set to 'default'. That results in the selection of the highest latency device with corresponding stutters and over-runs. The current requirement for selecting an output device is mildly annoying, but no where near as annoying as being forced to use a faulty device.
Re: d3d11: add a stub dll
On Tue, Dec 11, 2012 at 5:15 AM, Henri Verbeet wrote:
> On 11 December 2012 04:16, Austin English wrote:
>> + * Copyright 2012 The Wine Project
> I don't think that kind of thing really makes sense unless you also
> define "The Wine Project" as some kind of legal entity somewhere.
It's also done in some headers/other files. I didn't want to put my
name on it for so little code, but that's a trivial thing to change.
> +TRACE("(0x%p, %d, %p)\n", hinstDLL, fdwReason, lpvReserved);
> 0x%p is just wrong, and %d is at least questionable for a DWORD. I
> suspect this was just copied from somewhere else though.
The stub was created using winedump.
--
-Austin
Re: testbot: Fix the ConfigLocal.pl include path.
On Tue, 11 Dec 2012, Alexandre Julliard wrote:
> Francois Gouget writes:
>
> > @@ -88,12 +88,11 @@ $JobArchiveDays = 0;
> > if (!$::BuildEnv)
> > {
> >$::BuildEnv = 0;
> > - eval 'require "$::RootDir/ConfigLocal.pl";';
> > + eval 'require "$::RootDir/lib/WineTestBot/ConfigLocal.pl"';
[...]
> I had changed it because we don't want to have local config files inside
> the git checkout.
I see. It's for when the bin, lib and web directories are replaced with
symbolic links to directories belonging to another user. Here I had the
configuration file in that other user's account so that WineTestBot
could not modify it. But I'm fine with either way. I'll update the
instructions.
--
Francois Gouget
Re: RFC: Remove auto-scan of ALSA devices from winealsa.drv
On 11 December 2012 16:05, wrote: > Cost to users: > > Users with a working ALSA device "default" should experience no > drawback, only benefits. I believe this is the vast majority of users. > > Users that edit their ~/.asoundrc to define other devices without > simultaneously overriding !default will have to additionally edit the > Wine registry to name their working ALSA capture and render devices, > e.g. "asnoop" and "amix". > It will also pretty much just remove device selection on setup with multiple audio devices, which is actually fairly common these days with USB audio devices and HDMI outputs on graphics cards. I think the correct approach would to work with upstream ALSA to fix things, instead of just removing device enumeration.
RFC: Remove auto-scan of ALSA devices from winealsa.drv
Hi,
Here's my proposal:
winealsa shall stop enumerating ALSA devices. By default, it should
solely provide access to ALSA's default device adequately named "default".
The code that currently scans the registry
Software\Wine\Drivers\winealsa.drv\devices=... shall remain in place,
allowing a comma-separated list of alternate ALSA device names
for those setups where "default" is not the preferred ALSA choice.
Rationale:
ALSA front-ends (pulse, dmix etc.) compete for the few back-ends (one
or 2 sound cards or audio plugs). Some front-ends cling to the back
ends even when unused. This has been known to cause random bugs.
- Test suites failing randomly ("resource temporarily unavailable")
e.g. bug #28109, #28048
or randomly performing more or less tests, depending on whether the
app managed to open the device or not.
- Sound stopping in apps (I believe when transitioning from intros to
the main menu or game, at which time a rescan of all audio devices
happens as mmdevapi is reopened).
Nobody found an elegant way to robustly enumerate ALSA devices (some
apps hard-wire common names...), so let's simply avoid the issue entirely.
On many systems, the current code enumerates a quite uninteresting set
of devices, e.g. "default", "plughw:0" and "plughw:4". These days,
who wants hw:0 without mixing? I never heard any sound from "hw:4".
The current code fails to enumerate some existing devices like "plug:dmix".
OTOH, enumerating solely "default" or the user's hand-edited list of devices
is expected to provide repeatable and best results.
Cost to users:
Users with a working ALSA device "default" should experience no
drawback, only benefits. I believe this is the vast majority of users.
Users that edit their ~/.asoundrc to define other devices without
simultaneously overriding !default will have to additionally edit the
Wine registry to name their working ALSA capture and render devices,
e.g. "asnoop" and "amix".
winecfg may or may not provide a GUI for editing this.
This would be similar to most multimedia players allowing
some way to change the ALSA device to use.
It's a pity that the registry scheme AFAIK does not allow per-app
audio settings, but that's a completely unrelated issue.
Cost of non-adoption:
Users and audio devs remain annoyed. Bug reports remain open.
Unpredictable and erratic loss of sound possible each time mmdevapi
rescans the ALSA devices.
Please comment,
Jörg Höhle
Re: wininet: Don't perform revocation checks when verifying a certificate.
On Tue, 2012-12-11 at 14:52 +0100, Jacek Caban wrote: > On 12/11/12 09:45, Hans Leidekker wrote: > > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test which shows > > that > > revocation checks fail for the certificate on outlook.com when passed > > straight > > to CertVerifyRevocation. The reason is that a CRL link specified in the > > certificate does not resolve. > > > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test which makes > > a secure connection to outlook.com from wininet and shows that this > > succeeds. > > > > My conclusion is that native wininet doesn't perform revocation checks. > > Your tests prove that we should relax our verification on > CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that > revocation checks are not made, a test with truly revoked cert would be > needed. True, though to perform the revocation check the CRL has to be retrieved and my tests with wireshark didn't show any signs of that.
Re: wininet: Don't perform revocation checks when verifying a certificate.
Hi Hans, On 12/11/12 09:45, Hans Leidekker wrote: > https://testbot.winehq.org/JobDetails.pl?Key=23300 is a test which shows that > revocation checks fail for the certificate on outlook.com when passed straight > to CertVerifyRevocation. The reason is that a CRL link specified in the > certificate does not resolve. > > https://testbot.winehq.org/JobDetails.pl?Key=23301 is a test which makes > a secure connection to outlook.com from wininet and shows that this succeeds. > > My conclusion is that native wininet doesn't perform revocation checks. Your tests prove that we should relax our verification on CERT_TRUST_IS_OFFLINE_REVOCATION or something similar. To prove that revocation checks are not made, a test with truly revoked cert would be needed. Jacek
Re: d3d11: add a stub dll
On 11 December 2012 04:16, Austin English wrote:
> + * Copyright 2012 The Wine Project
I don't think that kind of thing really makes sense unless you also
define "The Wine Project" as some kind of legal entity somewhere.
> +TRACE("(0x%p, %d, %p)\n", hinstDLL, fdwReason, lpvReserved);
0x%p is just wrong, and %d is at least questionable for a DWORD. I
suspect this was just copied from somewhere else though.
Re: testbot: Fix the ConfigLocal.pl include path.
Francois Gouget writes:
> @@ -88,12 +88,11 @@ $JobArchiveDays = 0;
> if (!$::BuildEnv)
> {
>$::BuildEnv = 0;
> - eval 'require "$::RootDir/ConfigLocal.pl";';
> + eval 'require "$::RootDir/lib/WineTestBot/ConfigLocal.pl"';
>if ($@)
>{
> -print STDERR "Please create a valid $::RootDir/ConfigLocal.pl, " .
> -"use $::RootDir/lib/WineTestBot/ConfigLocalTemplate.pl as
> template\n";
> -exit;
> +print STDERR "Please create a valid
> $::RootDir/lib/WineTestBot/ConfigLocal.pl file, use ConfigLocalTemplate.pl as
> template\n";
> +exit(1);
I had changed it because we don't want to have local config files inside
the git checkout.
--
Alexandre Julliard
julli...@winehq.org
Re: [PATCH 1/3] include: Define FIELD_OFFSET to the standard offsetof macro
On 12/11/2012 10:20 AM, Dmitry Timoshkov wrote: > Michael Stefaniuc wrote: > >> On 12/10/2012 07:37 PM, Amine Khaldi wrote: >>> This prevents the undefined behavior (null pointer dereference) >>> diagnostics (clang with ubsan checks for example). >> This is a bug in clang. There is no null pointer dereference. >> Afair gcc tried to pull this trick too but got educated about their error. > > What's an advantage of these patches except of catering broken compilers/ > checkers? I figure you mean the 2nd try which uses offsetof unconditionally. The advantage is that it replaces the cast orgy with the C89 standard construct thus it easier to read. Appeasing the still wrong clang check is just a side effect. bye michael
Re: [PATCH 1/3] include: Define FIELD_OFFSET to the standard offsetof macro
Michael Stefaniuc wrote: > On 12/10/2012 07:37 PM, Amine Khaldi wrote: > > This prevents the undefined behavior (null pointer dereference) > > diagnostics (clang with ubsan checks for example). > This is a bug in clang. There is no null pointer dereference. > Afair gcc tried to pull this trick too but got educated about their error. What's an advantage of these patches except of catering broken compilers/ checkers? -- Dmitry.
