Due to my family presence, I need to stop the Test KVM AHK bot during this week end.I have good news regarding CORE-14536 and sound in general.
"I'll be back." Kind regards, Sylvain Petreolle
Le Vendredi 27 avril 2018 14h46, Serge Gautherie <reactos_serge_161107(a)gautherie.fr> a écrit :
Hello Sylvain,
Test KVM AHK is offline since
"about 1 day ago (2018-Apr-25 18:27:36)".
Thanks.
--
1) I thought certain MDL fast I/O routines can get called at DISPATCH_LEVEL
2) For perf reasons, causing a page-fault in the middle of a "fast I/O" is
probably a bad idea.
That being said -- these concerns seem silly since we are talking about the
"Null" driver which... is entirely PAGED_CODE and clearly neither #1 or #2
are actual issues.
In Windows, the fast I/o table is part of the DEVICE_OBJECT itself. Not
sure why our driver allocates a separate pool allocation to begin with.
Best regards,
Alex Ionescu
On Mon, Apr 23, 2018 at 1:30 AM, Ged Murphy <gedmurphy.maillists(a)gmail.com>
wrote:
> They're pagable in NT6, I don't know whether that's the case in NT5.
>
> Change looks good to me, I'm not really sure why Alex used NNP?
> Perhaps our kernel does/did things slightly differently back when he wrote
> the Null driver?
>
> Ged.
>
> -----Original Message-----
> From: Ros-dev <ros-dev-bounces(a)reactos.org> On Behalf Of Thomas Faber
> Sent: Monday, 23 April 2018 07:49
> To: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
> Cc: ros-dev(a)reactos.org
> Subject: Re: [ros-dev] [ros-diffs] 01/01: [NULL] Additions for the Null
> driver.
>
> On 2018-04-22 22:23, Hermès Bélusca-Maïto wrote:
> > diff --git a/drivers/base/null/null.c b/drivers/base/null/null.c index
> > 610e886ddd..0d4ed541de 100644
> > --- a/drivers/base/null/null.c
> > +++ b/drivers/base/null/null.c
> > @@ -181,26 +199,16 @@ DriverEntry(IN PDRIVER_OBJECT DriverObject,
> > DriverObject->MajorFunction[IRP_MJ_READ] = NullDispatch;
> > DriverObject->MajorFunction[IRP_MJ_LOCK_CONTROL] = NullDispatch;
> > DriverObject->MajorFunction[IRP_MJ_QUERY_INFORMATION] =
> > NullDispatch;
> > + DriverObject->DriverUnload = NullUnload;
> >
> > - /* Allocate the fast I/O dispatch table */
> > - FastIoDispatch = ExAllocatePoolWithTag(NonPagedPool,
> > - sizeof(FAST_IO_DISPATCH),
> > - 'llun');
> > - if (!FastIoDispatch)
> > - {
> > - /* Failed, cleanup */
> > - IoDeleteDevice(DeviceObject);
> > - return STATUS_INSUFFICIENT_RESOURCES;
> > - }
> > -
> > - /* Initialize it */
> > - RtlZeroMemory(FastIoDispatch, sizeof(FAST_IO_DISPATCH));
> > - FastIoDispatch->SizeOfFastIoDispatch = sizeof(FAST_IO_DISPATCH);
> > + /* Initialize the fast I/O dispatch table */
> > + RtlZeroMemory(&FastIoDispatch, sizeof(FastIoDispatch));
> > + FastIoDispatch.SizeOfFastIoDispatch = sizeof(FastIoDispatch);
> >
> > /* Setup our pointers */
> > - FastIoDispatch->FastIoRead = NullRead;
> > - FastIoDispatch->FastIoWrite = NullWrite;
> > - DriverObject->FastIoDispatch = FastIoDispatch;
> > + FastIoDispatch.FastIoRead = NullRead;
> > + FastIoDispatch.FastIoWrite = NullWrite;
> > + DriverObject->FastIoDispatch = &FastIoDispatch;
>
>
> Are you sure FAST_IO_DISPATCH is allowed to be pageable? It seems to only
> be used at low IRQLs, so it seems logical. However I see it allocated
> nonpaged everywhere else and can't seem to find definitive documentation on
> the subject.
> (And yes, most filesystem drivers use a static structure, but those
> drivers don't use MmPageEntireDriver)
>
> Thanks,
> Thomas
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
Hi all!
Let me invite you to the April 2018 meeting, taking place this Thursday,
April 26, 2018 at 19:00 UTC.
There have been no requests for additional agenda points yet and I'm not
aware of any developments of the outstanding points of last meeting, so
right now the agenda only includes the Status Updates.
If there is something you want discussed this month, please reply to
this mail.
As waiting for every developer's Status Report still wastes too much
time each month, I'm asking everyone to prepare a short text in advance.
Unlike previous meetings, let's all try to post our status updates
simultaneously this time and let the IRC server handle the congestion.
This may make some reports harder to read, but we should be able to move
on much quicker.
Best regards,
Colin
Hey Eric,
On 2018-04-02 12:58, Eric Kohl wrote:
> - RtlStringCbPrintfW(strbuf, sizeof(strbuf), L"%d:%d:%d", hours, minutes, seconds);
> + swprintf(szBuffer, L"%02d:%02d:%02d", iHours, iMinutes, iSeconds);
Unfortunately I must disagree with this change.
Buffer overflows are a big enough threat that code review and
static analysis are not generally considered sufficient to protect
against them.
So it's best practice for new code to always verify sizes at run-time,
and never to use s(w)print.
Best regards,
Thomas
PS: from what I see, iHours can be as large as 1193046, which won't
fit in 2 digits
So to be clear, while the kernel still has tons of incompatible code and
issues to barely run as a Win2003-compatible kernel, whenever there's an NT
design decision you disagree with, you're going to be rewriting the little
bit of code that _does work well_ to work contrary to how NT works? Did I
get that right?
Good luck.
Best regards,
Alex Ionescu
On Mon, Apr 2, 2018 at 6:48 AM, Hermès BÉLUSCA-MAÏTO <hermes.belusca(a)sfr.fr>
wrote:
> Yes, to only allow programs that REALLY REALLY REALLY REALLY ….. need to
> do so to trigger the hard-error “shutdown” BSOD from user-mode to do so,
> and these programs would better be only those that run only in SYSTEM
> rights, and more exactly these include CSRSS, WINLOGON and SMSS when
> something very bad happen to them.
>
> I would not appreciate, for example, that when I run a program under a
> not-so privileged account (like, some random user account) that has just
> the shutdown privilege to shut the computer down properly, that this
> program suddently “BSODS” my machine.
>
> To these programs, I say “f$ck these!”
>
>
>
> Regards,
>
> Hermès
>
>
>
> *De :* Ros-dev [mailto:ros-dev-bounces@reactos.org] *De la part de* Alex
> Ionescu
> *Envoyé :* lundi 2 avril 2018 04:20
> *À :* ReactOS Development List; Hermès Bélusca-Maïto
> *Cc :* Linda Wang
> *Objet :* Re: [ros-dev] [ros-diffs] 02/08: [NTOSKRNL] Forbid processes
> without the Tcb prvilege to perform a user-mode hard-error BSOD.
>
>
>
> Is there a point to this blatant behavior change?
>
>
> Best regards,
> Alex Ionescu
>
>
>
> On Sun, Apr 1, 2018 at 3:04 PM, Hermès Bélusca-Maïto <
> hermes.belusca-maito(a)reactos.org> wrote:
>
> https://git.reactos.org/?p=reactos.git;a=commitdiff;h=
> f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
>
> commit f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
> Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
> AuthorDate: Sun Apr 1 14:46:19 2018 +0200
> Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
> CommitDate: Sun Apr 1 22:39:31 2018 +0200
>
> [NTOSKRNL] Forbid processes without the Tcb prvilege to perform a
> user-mode hard-error BSOD.
> ---
> ntoskrnl/ex/harderr.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/ntoskrnl/ex/harderr.c b/ntoskrnl/ex/harderr.c
> index 84f409a1bb..a5200e3e74 100644
> --- a/ntoskrnl/ex/harderr.c
> +++ b/ntoskrnl/ex/harderr.c
> @@ -132,8 +132,18 @@ ExpRaiseHardError(IN NTSTATUS ErrorStatus,
> /* Check if this error will shutdown the system */
> if (ValidResponseOptions == OptionShutdownSystem)
> {
> - /* Check for privilege */
> - if (!SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
> + /*
> + * Check if we have the privileges.
> + *
> + * NOTE: In addition to the Shutdown privilege we also check
> whether
> + * the caller has the Tcb privilege. The purpose is to allow only
> + * SYSTEM processes to "shutdown" the system on hard errors (BSOD)
> + * while forbidding regular processes to do so. This behaviour
> differs
> + * from Windows, where any user-mode process, as soon as it has
> the
> + * Shutdown privilege, can trigger a hard-error BSOD.
> + */
> + if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode) ||
> + !SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
> {
> /* No rights */
> *Response = ResponseNotHandled;
>
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
>
If I remember correctly you can make shutdowns delayed of many days on Windows (using the InitiateSystemShutdown(Ex) function), in which case the 2-digit hour won't work at all.
Best,
Hermès
> -----Message d'origine-----
> De : Ros-dev [mailto:ros-dev-bounces@reactos.org] De la part de Thomas
> Faber
> Envoyé : lundi 2 avril 2018 14:13
> À : Eric Kohl
> Cc : ros-dev(a)reactos.org
> Objet : Re: [ros-dev] [ros-diffs] 01/01: [WINLOGON] Clean up part 2 - Replace
> the UNICODE_STRING usMessage by a PWSTR pszMessage. - Use the
> "%02d:%02d:%02d" time format and get rid of the safe string printf because
> the string will NEVER be longer than 8 character
>
> Hey Eric,
>
> On 2018-04-02 12:58, Eric Kohl wrote:
> > - RtlStringCbPrintfW(strbuf, sizeof(strbuf), L"%d:%d:%d", hours, minutes,
> seconds);
> > + swprintf(szBuffer, L"%02d:%02d:%02d", iHours, iMinutes,
> > + iSeconds);
>
> Unfortunately I must disagree with this change.
>
> Buffer overflows are a big enough threat that code review and static analysis
> are not generally considered sufficient to protect against them.
> So it's best practice for new code to always verify sizes at run-time, and
> never to use s(w)print.
>
> Best regards,
> Thomas
>
> PS: from what I see, iHours can be as large as 1193046, which won't
> fit in 2 digits
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev(a)reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
Is there a point to this blatant behavior change?
Best regards,
Alex Ionescu
On Sun, Apr 1, 2018 at 3:04 PM, Hermès Bélusca-Maïto <
hermes.belusca-maito(a)reactos.org> wrote:
> https://git.reactos.org/?p=reactos.git;a=commitdiff;h=
> f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
>
> commit f0729b30bb79d6f538cf2b9578ff8ebe7989f8d3
> Author: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
> AuthorDate: Sun Apr 1 14:46:19 2018 +0200
> Commit: Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
> CommitDate: Sun Apr 1 22:39:31 2018 +0200
>
> [NTOSKRNL] Forbid processes without the Tcb prvilege to perform a
> user-mode hard-error BSOD.
> ---
> ntoskrnl/ex/harderr.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/ntoskrnl/ex/harderr.c b/ntoskrnl/ex/harderr.c
> index 84f409a1bb..a5200e3e74 100644
> --- a/ntoskrnl/ex/harderr.c
> +++ b/ntoskrnl/ex/harderr.c
> @@ -132,8 +132,18 @@ ExpRaiseHardError(IN NTSTATUS ErrorStatus,
> /* Check if this error will shutdown the system */
> if (ValidResponseOptions == OptionShutdownSystem)
> {
> - /* Check for privilege */
> - if (!SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
> + /*
> + * Check if we have the privileges.
> + *
> + * NOTE: In addition to the Shutdown privilege we also check
> whether
> + * the caller has the Tcb privilege. The purpose is to allow only
> + * SYSTEM processes to "shutdown" the system on hard errors (BSOD)
> + * while forbidding regular processes to do so. This behaviour
> differs
> + * from Windows, where any user-mode process, as soon as it has
> the
> + * Shutdown privilege, can trigger a hard-error BSOD.
> + */
> + if (!SeSinglePrivilegeCheck(SeTcbPrivilege, PreviousMode) ||
> + !SeSinglePrivilegeCheck(SeShutdownPrivilege, PreviousMode))
> {
> /* No rights */
> *Response = ResponseNotHandled;
>
>