Date: Thu, 07 Jun 2007 21:49:06 -0700 From: Nate Lawson <nate@root.org> To: Hidetoshi Shimokawa <simokawa@FreeBSD.ORG> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/dev/firewire firewirereg.h fwohci.c sbp.c Message-ID: <4668DFC2.1070006@root.org> In-Reply-To: <626eb4530706071728w1a8d0f5fi198e5e708777498e@mail.gmail.com> References: <20070607132053.974EF16A584@hub.freebsd.org> <46683958.2060700@root.org> <626eb4530706071728w1a8d0f5fi198e5e708777498e@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hidetoshi Shimokawa wrote:
> On 6/8/07, Nate Lawson <nate@root.org> wrote:
>> Hidetoshi Shimokawa wrote:
>> > simokawa 2007-06-07 13:20:49 UTC
>> >
>> > FreeBSD src repository
>> >
>> > Modified files:
>> > sys/dev/firewire firewirereg.h fwohci.c sbp.c
>> > Log:
>> > Add a tunable hw.firewire.phydma_enable.
>> >
>> > This is enabled by default. It should be disabled for
>> > those who are uneasy with peeking/poking from FireWire.
>> >
>> > Please note sbp(4) and dcons(4) over FireWire need
>> > this feature.
>> >
>> > Revision Changes Path
>> > 1.48 +1 -0 src/sys/dev/firewire/firewirereg.h
>> > 1.91 +11 -6 src/sys/dev/firewire/fwohci.c
>> > 1.94 +7 -0 src/sys/dev/firewire/sbp.c
>> >
>> >
>> > Index: src/sys/dev/firewire/firewirereg.h
>> > diff -u src/sys/dev/firewire/firewirereg.h:1.47
>> src/sys/dev/firewire/firewirereg.h:1.48
>> > --- src/sys/dev/firewire/firewirereg.h:1.47 Wed Jun 6 14:31:36 2007
>> > +++ src/sys/dev/firewire/firewirereg.h Thu Jun 7 13:20:48 2007
>> > @@ -299,6 +299,7 @@
>> >
>> > extern int firewire_debug;
>> > extern devclass_t firewire_devclass;
>> > +extern int firewire_phydma_enable;
>> >
>> > #ifdef __DragonFly__
>> > #define FWPRI PCATCH
>> > Index: src/sys/dev/firewire/fwohci.c
>> > diff -u src/sys/dev/firewire/fwohci.c:1.90
>> src/sys/dev/firewire/fwohci.c:1.91
>> > --- src/sys/dev/firewire/fwohci.c:1.90 Wed Jun 6 14:31:36 2007
>> > +++ src/sys/dev/firewire/fwohci.c Thu Jun 7 13:20:48 2007
>> > @@ -79,9 +79,13 @@
>> > #undef OHCI_DEBUG
>> >
>> > static int nocyclemaster = 0;
>> > +int firewire_phydma_enable = 1;
>> > SYSCTL_DECL(_hw_firewire);
>> > SYSCTL_INT(_hw_firewire, OID_AUTO, nocyclemaster, CTLFLAG_RW,
>> &nocyclemaster, 0,
>> > "Do not send cycle start packets");
>> > +SYSCTL_INT(_hw_firewire, OID_AUTO, phydma_enable, CTLFLAG_RW,
>> > + &firewire_phydma_enable, 1, "Allow physical request DMA from
>> firewire");
>> > +TUNABLE_INT("hw.firewire.phydma_enable", &firewire_phydma_enable);
>> >
>> > static char dbcode[16][0x10]={"OUTM", "OUTL","INPM","INPL",
>> > "STOR","LOAD","NOP ","STOP",};
>> > @@ -1862,12 +1866,13 @@
>> >
>> > /* Allow async. request to us */
>> > OWRITE(sc, OHCI_AREQHI, 1 << 31);
>> > - /* XXX insecure ?? */
>> > - /* allow from all nodes */
>> > - OWRITE(sc, OHCI_PREQHI, 0x7fffffff);
>> > - OWRITE(sc, OHCI_PREQLO, 0xffffffff);
>> > - /* 0 to 4GB regison */
>> > - OWRITE(sc, OHCI_PREQUPPER, 0x10000);
>> > + if (firewire_phydma_enable) {
>> > + /* allow from all nodes */
>> > + OWRITE(sc, OHCI_PREQHI, 0x7fffffff);
>> > + OWRITE(sc, OHCI_PREQLO, 0xffffffff);
>> > + /* 0 to 4GB region */
>> > + OWRITE(sc, OHCI_PREQUPPER, 0x10000);
>> > + }
>> > /* Set ATRetries register */
>> > OWRITE(sc, OHCI_ATRETRY, 1<<(13+16) | 0xfff);
>>
>> I see this runs each time a PHY_SID intr arrives. Is that enough to
>> really prevent access if a pre-existing host on the bus requests DMA? I
>> don't think an intr is generated in that case.
>>
>> Should this be a separate function run directly by the sysctl that sets
>> the filters immediately?
>
> Yes, that's true.
>
> I recommend you to put it in loader.conf
Since someone who is setting this is concerned about security, should
this at least be documented? I'd prefer that the routine be changed to
set the filter registers synchronously with the sysctl.
--
Nate
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4668DFC2.1070006>