Re: Test needed: ehci(4) suspend/resume rework
On 01/05/13(Wed) 17:44, Ted Unangst wrote:
On Sun, Apr 28, 2013 at 15:44, Martin Pieuchot wrote:
Diff below is a rework of the suspend/resume logic in ehci(4).
In case this diff doesn't help or if you have a problem when resuming,
I left an #ifdef 0 block in the DVACT_RESUME. Try enabling it and tell
me if it changes something.
Got around to testing this. Now everything works. It still prints
echi_idone about 100 times after resume, but it doesn't print it
forever.
I'd say the diff works, but only with the reset in the resume case as
well.
Ok so, here's an updated diff with the reset enable in the resume case.
Please test and report to me.
Martin
Index: sys/dev/usb/ehci.c
===
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.131
diff -u -p -r1.131 ehci.c
--- sys/dev/usb/ehci.c 19 Apr 2013 08:58:53 - 1.131
+++ sys/dev/usb/ehci.c 2 May 2013 07:37:01 -
@@ -353,22 +353,10 @@ ehci_init(struct ehci_softc *sc)
sc-sc_bus.usbrev = USBREV_2_0;
- /* Reset the controller */
DPRINTF((%s: resetting\n, sc-sc_bus.bdev.dv_xname));
- EOWRITE4(sc, EHCI_USBCMD, 0); /* Halt controller */
- usb_delay_ms(sc-sc_bus, 1);
- EOWRITE4(sc, EHCI_USBCMD, EHCI_CMD_HCRESET);
- for (i = 0; i 100; i++) {
- usb_delay_ms(sc-sc_bus, 1);
- hcr = EOREAD4(sc, EHCI_USBCMD) EHCI_CMD_HCRESET;
- if (!hcr)
- break;
- }
- if (hcr) {
- printf(%s: reset timeout\n,
- sc-sc_bus.bdev.dv_xname);
- return (USBD_IOERROR);
- }
+ err = ehci_reset(sc);
+ if (err)
+ return (err);
/* frame list size at default, read back what we got and use that */
switch (EHCI_CMD_FLS(EOREAD4(sc, EHCI_USBCMD))) {
@@ -1032,6 +1020,8 @@ ehci_detach(struct ehci_softc *sc, int f
timeout_del(sc-sc_tmo_intrlist);
+ ehci_reset(sc);
+
usb_delay_ms(sc-sc_bus, 300); /* XXX let stray task complete */
/* XXX free other data structures XXX */
@@ -1044,7 +1034,7 @@ int
ehci_activate(struct device *self, int act)
{
struct ehci_softc *sc = (struct ehci_softc *)self;
- u_int32_t cmd, hcr;
+ u_int32_t cmd, hcr, cparams;
int i, rv = 0;
switch (act) {
@@ -1054,94 +1044,73 @@ ehci_activate(struct device *self, int a
case DVACT_SUSPEND:
sc-sc_bus.use_polling++;
- for (i = 1; i = sc-sc_noport; i++) {
- cmd = EOREAD4(sc, EHCI_PORTSC(i));
- if ((cmd (EHCI_PS_PO|EHCI_PS_PE)) == EHCI_PS_PE)
- EOWRITE4(sc, EHCI_PORTSC(i),
- cmd | EHCI_PS_SUSP);
- }
-
- sc-sc_cmd = EOREAD4(sc, EHCI_USBCMD);
- cmd = sc-sc_cmd ~(EHCI_CMD_ASE | EHCI_CMD_PSE);
+ /*
+* First tell the host to stop processing Asynchronous
+* and Periodic schedules.
+*/
+ cmd = EOREAD4(sc, EHCI_USBCMD) ~(EHCI_CMD_ASE | EHCI_CMD_PSE);
EOWRITE4(sc, EHCI_USBCMD, cmd);
-
for (i = 0; i 100; i++) {
+ usb_delay_ms(sc-sc_bus, 1);
hcr = EOREAD4(sc, EHCI_USBSTS)
(EHCI_STS_ASS | EHCI_STS_PSS);
if (hcr == 0)
break;
-
- usb_delay_ms(sc-sc_bus, 1);
}
if (hcr != 0)
- printf(%s: reset timeout\n,
+ printf(%s: disable schedules timeout\n,
sc-sc_bus.bdev.dv_xname);
- cmd = ~EHCI_CMD_RS;
- EOWRITE4(sc, EHCI_USBCMD, cmd);
-
- for (i = 0; i 100; i++) {
- hcr = EOREAD4(sc, EHCI_USBSTS) EHCI_STS_HCH;
- if (hcr == EHCI_STS_HCH)
- break;
-
- usb_delay_ms(sc-sc_bus, 1);
- }
- if (hcr != EHCI_STS_HCH)
- printf(%s: config timeout\n,
- sc-sc_bus.bdev.dv_xname);
+ /*
+* Then reset the host as if it was a shutdown.
+*
+* All USB devices are disconnected/reconnected during
+* a suspend/resume cycle so keep it simple.
+*/
+ ehci_reset(sc);
sc-sc_bus.use_polling--;
break;
case DVACT_POWERDOWN:
- ehci_shutdown(sc);
+ ehci_reset(sc);
break;
case DVACT_RESUME:
sc-sc_bus.use_polling++;
- /* restore things in case the bios sucks */
- EOWRITE4(sc, EHCI_CTRLDSSEGMENT, 0);
+
Re: DPI for pf(4)
On Wed, 1 May 2013, Franco Fichtner wrote: Not sure if that's a fitting comparison; and I know too little OSPF to answer. Let me try another route. The logic consists of an array of application detection functions, which can be invoked via their respective IP types. I don't like this approach at all - it leads to a proliferation (as demonstrated by your already long list) of kernel-side parsers that will be a maintenance, and possibly security, nightmare. The last thing we want it a rotting pile of protocol parsing code like wireshark. On May 1, 2013, at 1:14 AM, Ted Unangst t...@tedunangst.com wrote: My thoughts on the matter have always been that it would be cool to integrate bpf into pf (though other developers surely have other opinions). Then you get filtering for as many protocols as you care to write bpf matchers for. You mean externalising the DPI? People[1] have tried to work on such ideas, but the general drift is that there are not enough interested individuals in the field to drive second tier development for application detections. So if there is not enough interest to develop app protocol detectors/ disectors in bpf then why should C be any different? I find C to be quite flexible and empowering if one doesn't overcomplicate[2]. [2] https://github.com/fichtner/OpenDPI/blob/master/src/lib/protocols/ssl.c That's complicated and scary code for a kernel, e.g. multiple opportunities for unsigned overflow that don't seem to be checked for. I agree with tedu - it safer and more flexible to write parsers in bpf. If that isn't desirable then maybe we could consider some other automata classifier, but I think it is a bad, bad idea to do it in C. -d
Re: DPI for pf(4)
Hi Damien, On May 2, 2013, at 10:03 AM, Damien Miller d...@mindrot.org wrote: On Wed, 1 May 2013, Franco Fichtner wrote: Not sure if that's a fitting comparison; and I know too little OSPF to answer. Let me try another route. The logic consists of an array of application detection functions, which can be invoked via their respective IP types. I don't like this approach at all - it leads to a proliferation (as demonstrated by your already long list) of kernel-side parsers that will be a maintenance, and possibly security, nightmare. as stated before, breaking down complexity to the bare minimum is my requirement for this to be happening at all. You all get to be the judges. I'm just trying to work on something worth doing. The last thing we want it a rotting pile of protocol parsing code like wireshark. Case closed then? I don't know how to argue with that. On May 1, 2013, at 1:14 AM, Ted Unangst t...@tedunangst.com wrote: My thoughts on the matter have always been that it would be cool to integrate bpf into pf (though other developers surely have other opinions). Then you get filtering for as many protocols as you care to write bpf matchers for. You mean externalising the DPI? People[1] have tried to work on such ideas, but the general drift is that there are not enough interested individuals in the field to drive second tier development for application detections. So if there is not enough interest to develop app protocol detectors/ disectors in bpf then why should C be any different? Because it takes complexity out of the system for one. Plus, pf(4) is at the core of OpenBSD. There's not much noise about bpf(4) here. I find C to be quite flexible and empowering if one doesn't overcomplicate[2]. [2] https://github.com/fichtner/OpenDPI/blob/master/src/lib/protocols/ssl.c That's complicated and scary code for a kernel, e.g. multiple opportunities for unsigned overflow that don't seem to be checked for. You are absolutely right. And it's *not* my code, it was merely an example of how the TLS code can be broken down to the bare minimum[1]. I agree with tedu - it safer and more flexible to write parsers in bpf. If that isn't desirable then maybe we could consider some other automata classifier, but I think it is a bad, bad idea to do it in C. Again, I don't know how to argue with that. :) Kind regards, Franco [1] http://marc.info/?l=openbsd-techm=136739531914555w=2
Re: DPI for pf(4)
On Thu, 2 May 2013, Franco Fichtner wrote: as stated before, breaking down complexity to the bare minimum is my requirement for this to be happening at all. You all get to be the judges. I'm just trying to work on something worth doing. Well, bare minimum complexity per-protocol * large_number_of_protocols = a lot of complexity. The incentive is always going to be to add more protocols and never retire them. Also, doesn't IPPROTO_DIVERT or SO_BINDANY+SO_SPLICE allow you to do near zero-overhead DPI completely in userspace? -d
Re: [NEW] ugold(4) driver for Microdia's USB TEMPer variant
On 30/04/13(Tue) 17:39, Stuart Henderson wrote:
On 2013/04/30 15:06, Stuart Henderson wrote:
On 2013/03/31 17:56, SASANO Takayoshi wrote:
Hello,
I rewrote patched uthum(4) to new ugold(4) driver.
Thanks for advice by yuo@ and deraadt@.
Some comments inline.
Index: dev/usb/ugold.c
===
[...]
+/* Driver for Microdia's HID base TEMPer Temperature sensor */
+
+#include sys/param.h
+#include sys/proc.h
+#include sys/systm.h
+#include sys/kernel.h
+#include sys/malloc.h
+#include sys/device.h
+#include sys/conf.h
+#include sys/sensors.h
+
+#include dev/usb/usb.h
+#include dev/usb/usbhid.h
+#include dev/usb/usbdi.h
+#include dev/usb/usbdi_util.h
+#include dev/usb/usbdevs.h
+#include dev/usb/uhidev.h
+#include dev/usb/hid.h
Please make sure you include only the required headers, at least proc.h is
not needed.
+
+#ifdef USB_DEBUG
+#define UGOLD_DEBUG
+#endif
+
+#ifdef UGOLD_DEBUG
+int ugolddebug = 0;
+#define DPRINTFN(n, x) do { if (ugolddebug (n)) printf x; } while (0)
+#else
+#define DPRINTFN(n, x)
+#endif
+
+#define DPRINTF(x) DPRINTFN(0, x)
If you don't use the *N() variant, just keep it simple a only define the
DPRINTF() macro, this will also allow you to remove the ugolddebug
variable.
+
+/* Device types */
+#define UGOLD_TYPE_TEMPER0
+#define UGOLD_TYPE_UNKNOWN 0x
+
+/* sensors */
+#define UGOLD_TEMPER_INNER 0
+#define UGOLD_TEMPER_OUTER 1
+#define UGOLD_MAX_SENSORS2
+
+enum ugold_sensor_type {
+ UGOLD_SENSOR_UNKNOWN,
+ UGOLD_SENSOR_DS75,
+ UGOLD_SENSOR_MAXTYPES,
+};
+
+static const char * const ugold_sensor_type_s[UGOLD_SENSOR_MAXTYPES] = {
+ unknown,
+ ds75/12bit,
+};
+
+static uint8_t cmd_led_off[2] =
+ { 0x01, 0x00 };
+static uint8_t cmd_get_offset[8] =
+ { 0x01, 0x82, 0x77, 0x01, 0x00, 0x00, 0x00, 0x00 };
+static uint8_t cmd_init[8] =
+ { 0x01, 0x86, 0xff, 0x01, 0x00, 0x00, 0x00, 0x00 };
+static uint8_t cmd_get_data[8] =
+ { 0x01, 0x80, 0x33, 0x01, 0x00, 0x00, 0x00, 0x00 };
+
+struct ugold_sensor {
+ struct ksensor sensor;
+ int cal_offset; /* 10mC or m%RH */
+ int attached;
+ enum ugold_sensor_type dev_type;
+};
+
+struct ugold_softc {
+ struct uhidevsc_hdev;
+ usbd_device_handle sc_udev;
+ u_char sc_dying;
You don't use sc_dying so you can probably remove it and the *activate()
function also.
+ uint16_t sc_flag;
This field is also unused.
+ int sc_device_type;
Because you're attaching only one kind of device there's no need for
this field. I suggest you to remove it and all the unneeded logic that
comes with it.
+ int sc_initialized;
+ int sc_num_sensors;
+
+ /* uhidev parameters */
+ size_t sc_flen; /* feature report length */
+ size_t sc_ilen; /* input report length */
+ size_t sc_olen; /* output report length */
+
+ uint8_t *sc_ibuf;
+
+ /* sensor framework */
+ struct ugold_sensor sc_sensor[UGOLD_MAX_SENSORS];
+ struct ksensordevsc_sensordev;
+ struct sensor_task *sc_sensortask;
+};
+
+const struct usb_devno ugold_devs[] = {
+ { USB_VENDOR_MICRODIA, USB_PRODUCT_MICRODIA_TEMPER },
+};
+#define ugold_lookup(v, p) usb_lookup(ugold_devs, v, p)
+
+int ugold_match(struct device *, void *, void *);
+void ugold_attach(struct device *, struct device *, void *);
+int ugold_detach(struct device *, int);
+int ugold_activate(struct device *, int);
+
+int ugold_issue_cmd(struct ugold_softc *, uint8_t *, int, int);
+int ugold_read_data(struct ugold_softc *);
+int ugold_init_device(struct ugold_softc *);
+void ugold_setup_sensors(struct ugold_softc *);
+int ugold_setup_device(struct ugold_softc *);
+
+void ugold_intr(struct uhidev *, void *, u_int);
+void ugold_refresh(void *);
+void ugold_refresh_temper(struct ugold_softc *);
+
+int ugold_ds75_temp(uint8_t, uint8_t);
+void ugold_print_sensorinfo(struct ugold_softc *, int);
+
+struct cfdriver ugold_cd = {
+ NULL, ugold, DV_DULL
+};
+
+const struct cfattach ugold_ca = {
+ sizeof(struct ugold_softc),
+ ugold_match,
+ ugold_attach,
+ ugold_detach,
+ ugold_activate,
+};
+
+int
+ugold_match(struct device *parent, void *match, void *aux)
+{
+ struct usb_attach_arg *uaa = aux;
+ struct uhidev_attach_arg *uha = (struct uhidev_attach_arg *)uaa;
+
+ if (ugold_lookup(uha-uaa-vendor, uha-uaa-product) == NULL)
+ return UMATCH_NONE;
+
+ return (UMATCH_VENDOR_PRODUCT);
+}
+
+void
+ugold_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct ugold_softc *sc = (struct ugold_softc *)self;
+ struct
Re: DPI for pf(4)
On 2013/05/02 18:03, Damien Miller wrote: I find C to be quite flexible and empowering if one doesn't overcomplicate[2]. [2] https://github.com/fichtner/OpenDPI/blob/master/src/lib/protocols/ssl.c That's complicated and scary code for a kernel, e.g. multiple opportunities for unsigned overflow that don't seem to be checked for. Here Franco is giving an example of the overcomplicated code that other dpi is using - this is not what he is proposing for PF...
Re: DPI for pf(4)
On Thu, May 02, 2013 at 10:35:19AM +0200, Franco Fichtner wrote: as stated before, breaking down complexity to the bare minimum is my requirement for this to be happening at all. You all get to be the judges. I'm just trying to work on something worth doing. The last thing we want it a rotting pile of protocol parsing code like wireshark. Case closed then? I don't know how to argue with that. IMHO, don't ask and don't argue. If you need DPI in pf (or whatever), write it *for you*, then use it for *your needs*. If one day you feel it could be useful to others, share the code and someone may like it. Speaking of complexity, OpenBSD already has plenty of complicated kernel code that could run in user-mode but it's in the kernel because it was easier that way, or the author thought it's faster that way or ports expect it to be that way. -- Alexandre
stop gcc warning about missing newlines at eof
upstream gcc stopped warning about missing newlines at eof
five years ago, here is a diff to do the same for our gcc3/gcc4.
Index: gcc/libcpp/lex.c
===
RCS file: /cvs/src/gnu/gcc/libcpp/lex.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 lex.c
--- gcc/libcpp/lex.c15 Oct 2009 17:11:30 - 1.1.1.1
+++ gcc/libcpp/lex.c2 May 2013 08:05:12 -
@@ -837,9 +837,6 @@ _cpp_get_fresh_line (cpp_reader *pfile)
{
/* Only warn once. */
buffer-next_line = buffer-rlimit;
- cpp_error_with_line (pfile, CPP_DL_PEDWARN,
pfile-line_table-highest_line,
- CPP_BUF_COLUMN (buffer, buffer-cur),
- no newline at end of file);
}
return_at_eof = buffer-return_at_eof;
Index: usr.bin/gcc/gcc/cpplex.c
===
RCS file: /cvs/src/gnu/usr.bin/gcc/gcc/cpplex.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 cpplex.c
--- usr.bin/gcc/gcc/cpplex.c29 Nov 2003 12:21:48 - 1.1.1.1
+++ usr.bin/gcc/gcc/cpplex.c2 May 2013 08:14:59 -
@@ -911,8 +911,6 @@ continue_after_nul (pfile)
{
/* Non-empty files should end in a newline. Don't warn
for command line and _Pragma buffers. */
- if (!buffer-from_stage3)
- cpp_error (pfile, DL_PEDWARN, no newline at end of file);
handle_newline (pfile);
}
Index: usr.bin/gcc/gcc/cpptrad.c
===
RCS file: /cvs/src/gnu/usr.bin/gcc/gcc/cpptrad.c,v
retrieving revision 1.1.1.2
diff -u -p -r1.1.1.2 cpptrad.c
--- usr.bin/gcc/gcc/cpptrad.c 24 Dec 2004 23:51:31 - 1.1.1.2
+++ usr.bin/gcc/gcc/cpptrad.c 2 May 2013 08:14:30 -
@@ -487,8 +487,6 @@ scan_out_logical_line (pfile, macro)
/* Premature end of file. Fake a new line. */
cur--;
- if (!pfile-buffer-from_stage3)
- cpp_error (pfile, DL_PEDWARN, no newline at end of file);
pfile-line++;
goto done;
Re: DPI for pf(4)
On May 2, 2013, at 10:45 AM, Damien Miller d...@mindrot.org wrote: On Thu, 2 May 2013, Franco Fichtner wrote: as stated before, breaking down complexity to the bare minimum is my requirement for this to be happening at all. You all get to be the judges. I'm just trying to work on something worth doing. Well, bare minimum complexity per-protocol * large_number_of_protocols = a lot of complexity. The incentive is always going to be to add more protocols and never retire them. I guess that's true for most software projects. Also, doesn't IPPROTO_DIVERT or SO_BINDANY+SO_SPLICE allow you to do near zero-overhead DPI completely in userspace? Wouldn't that mean pf.conf(5) syntax extensions cannot be implemented? It's not full-blown DPI analysis for extracting all kinds of events from a flow -- it's merely a tagging tool, and if that sits in user space, it's really not helpful except for logging / accounting. One could do that with a simple pcap(3) binding as well. Stuart made a good point for divert-packet being able to pick up applications without the need for any other information (ports, interfaces, addresses). I'm sorry for not being able to make it more clear at this time. Next step for me is to write a comprehensive description. In any case, the input on tech@ has been very helpful so far. Thanks guys! :) Franco
Re: DPI for pf(4)
On Thu, 2 May 2013, Franco Fichtner wrote: Well, bare minimum complexity per-protocol * large_number_of_protocols = a lot of complexity. The incentive is always going to be to add more protocols and never retire them. I guess that's true for most software projects. We try not to implement an effectively unbounded number of protocol parsers in the kernel. Also, doesn't IPPROTO_DIVERT or SO_BINDANY+SO_SPLICE allow you to do near zero-overhead DPI completely in userspace? Wouldn't that mean pf.conf(5) syntax extensions cannot be implemented? It doesn't mean that - you'd just need some way for userspace to signal information to pf. E.g add a SO_PF_TAG to set the pf tag. Then you could use some program that used SO_BINDANY to inspect the beginning of the session, set a pf tag using setsockopt, SO_SPLICE to avoid further need to copy the session in userspace and control the traffic in pf using the tagged keyword. It's not full-blown DPI analysis for extracting all kinds of events from a flow -- it's merely a tagging tool, and if that sits in user space, it's really not helpful except for logging / accounting. One could do that with a simple pcap(3) binding as well. Why not do the tagging in userspace using the existing facilities? -d
Re: DPI for pf(4)
On May 2, 2013, at 1:23 PM, Damien Miller d...@mindrot.org wrote: On Thu, 2 May 2013, Franco Fichtner wrote: Well, bare minimum complexity per-protocol * large_number_of_protocols = a lot of complexity. The incentive is always going to be to add more protocols and never retire them. I guess that's true for most software projects. We try not to implement an effectively unbounded number of protocol parsers in the kernel. Agreed. Let's put a hard limit on it. 5, 10, 20, 50? Also, doesn't IPPROTO_DIVERT or SO_BINDANY+SO_SPLICE allow you to do near zero-overhead DPI completely in userspace? Wouldn't that mean pf.conf(5) syntax extensions cannot be implemented? It doesn't mean that - you'd just need some way for userspace to signal information to pf. E.g add a SO_PF_TAG to set the pf tag. Then you could use some program that used SO_BINDANY to inspect the beginning of the session, set a pf tag using setsockopt, SO_SPLICE to avoid further need to copy the session in userspace and control the traffic in pf using the tagged keyword. That sounds a bit too complex as well, but would likely work. I'll read into this some more, thanks. It's not full-blown DPI analysis for extracting all kinds of events from a flow -- it's merely a tagging tool, and if that sits in user space, it's really not helpful except for logging / accounting. One could do that with a simple pcap(3) binding as well. Why not do the tagging in userspace using the existing facilities? Mainly to avoid any kind of introduction of latency, buffering, asynchronous behaviour, packet reordering, not invoking the scheduler, avoiding cache line bouncing, and being generally prone to multithreading issues in a perfect world where multiple CPUs could drive the networking stack. Also not having to reimplement certain packet parsing code, state tracking, and so on and so forth. Look, I have written all that stuff in user space, but redundancy and complicated architectures are not suitable for forwarding large loads of traffic. User space is that magical place that can do anything, even throw off your packet throughput by invoking a syscall to pull the current time stamp. Moving implementations to user space does not necessarily make them better or less of a problem. That's my concern. :) Franco
Behaviour of ksh(1)
Hi, you should try echo BEFORE unset KSH_VERSION echo AFTER in your ~/.shrc (note: i include that from within my ~/.profile and let it auto-protect itself against multiple inclusions since my shell startup scripts are multi-platform multi-shell, all in two). Note it doesn't matter if you use eval 'unset KSH_VERSION' and that i consider a bug. Neither mksh(1) nor other Korn shells i've tried (and no Bourne shells at all) show this behaviour. Thanks and ciao --steffen
Re: DPI for pf(4)
On Thu, 2 May 2013, Franco Fichtner wrote: Moving implementations to user space does not necessarily make them better or less of a problem. The big difference is that its possible to sandbox a userspace implementation so that small integer overflow bugs or length checking failures don't become arbitrary kmem reads or, worse, RCE. -d
Re: DPI for pf(4)
On Thu, 2 May 2013, Franco Fichtner wrote: OK, the implementation only pulls a couple of bytes from the packet's payload. It will never pull bytes that are not verified. It will never allocate anything. It will never test against something that's neither hard-coded nor available in the range of the approved payload. It will never return more than unsigned int with a number describing the actual application. It will never manipulate any input value, lest of all the packet itself. It will never run into endless loops. And I'll gladly zap everything that could still considered be a potential risk. You've just described bpf, right down to no endless loops and the amount of data it returns. For a little more code that it takes to write one packet parser (basically: loading bpf rules from pf and making the bpf_filter()'s return value available to it) you get everything you described above and more. -d
Re: DPI for pf(4)
On May 2, 2013, at 2:40 PM, Damien Miller d...@mindrot.org wrote: On Thu, 2 May 2013, Franco Fichtner wrote: Moving implementations to user space does not necessarily make them better or less of a problem. The big difference is that its possible to sandbox a userspace implementation so that small integer overflow bugs or length checking failures don't become arbitrary kmem reads or, worse, RCE. OK, the implementation only pulls a couple of bytes from the packet's payload. It will never pull bytes that are not verified. It will never allocate anything. It will never test against something that's neither hard-coded nor available in the range of the approved payload. It will never return more than unsigned int with a number describing the actual application. It will never manipulate any input value, lest of all the packet itself. It will never run into endless loops. And I'll gladly zap everything that could still considered be a potential risk. Parsing TCP options is still more complex than what this particular DPI code is supposed to be doing. This comes from personal experience. ;) IMHO, the only issue that remains is a potentially unlimited number of applications. That's a strong point against the idea. Franco
Re: Behaviour of ksh(1)
On Thu, May 02, 2013 at 13:12, Steffen Daode Nurpmeso wrote: Hi, you should try echo BEFORE unset KSH_VERSION echo AFTER ksh: unset: KSH_VERSION is read only So don't unset it?
Re: DPI for pf(4)
On May 2, 2013, at 3:20 PM, Damien Miller d...@mindrot.org wrote: On Thu, 2 May 2013, Franco Fichtner wrote: OK, the implementation only pulls a couple of bytes from the packet's payload. It will never pull bytes that are not verified. It will never allocate anything. It will never test against something that's neither hard-coded nor available in the range of the approved payload. It will never return more than unsigned int with a number describing the actual application. It will never manipulate any input value, lest of all the packet itself. It will never run into endless loops. And I'll gladly zap everything that could still considered be a potential risk. You've just described bpf, right down to no endless loops and the amount of data it returns. For a little more code that it takes to write one packet parser (basically: loading bpf rules from pf and making the bpf_filter()'s return value available to it) you get everything you described above and more. I yield. I'm working on making DPI more human-readable and maintainable, and struct bpf_insn is not an option for me, personally. Worse still, searching for bpf+dpi in google already brings up this mail thread as a top ten hit, which may be a good indicator of how successful this approach has been the last couple of years. ;) Franco
Re: DPI for pf(4)
fOn Thu, May 02, 2013 at 04:03:05PM +0200, Franco Fichtner wrote: On May 2, 2013, at 3:20 PM, Damien Miller d...@mindrot.org wrote: On Thu, 2 May 2013, Franco Fichtner wrote: OK, the implementation only pulls a couple of bytes from the packet's payload. It will never pull bytes that are not verified. It will never allocate anything. It will never test against something that's neither hard-coded nor available in the range of the approved payload. It will never return more than unsigned int with a number describing the actual application. It will never manipulate any input value, lest of all the packet itself. It will never run into endless loops. And I'll gladly zap everything that could still considered be a potential risk. You've just described bpf, right down to no endless loops and the amount of data it returns. For a little more code that it takes to write one packet parser (basically: loading bpf rules from pf and making the bpf_filter()'s return value available to it) you get everything you described above and more. I yield. I'm working on making DPI more human-readable and maintainable, and struct bpf_insn is not an option for me, personally. libpcap has a fairly simple parser to turn expressions into bpf instructions. It is used by tcpdump. Worse still, searching for bpf+dpi in google already brings up this mail thread as a top ten hit, which may be a good indicator of how successful this approach has been the last couple of years. ;) Franco
UPDATE: xf86-input-synaptics 1.7.0
Hi, This update xf86-input-synaptics to the latest release 1.7.0. http://koba.devio.us/distfiles/xf86-input-synaptics-1.7.0.diff Tested on amd64 and i386. Comments ? OK ? -- Alexandr Shadchin
Re: Behaviour of ksh(1)
Ted Unangst t...@tedunangst.com wrote: |On Thu, May 02, 2013 at 13:12, Steffen Daode Nurpmeso wrote: | Hi, you should try | | echo BEFORE | unset KSH_VERSION | echo AFTER | |ksh: unset: KSH_VERSION is read only | |So don't unset it? Yep it's nasty hah. Still, you get BEFORE ksh: unset: KSH_VERSION is read only and .profile is read by an interactive shell, so evaluation must not stop here, as has been revealed on the mksh(1) list -- POSIX says (XCU, chapter 2: Shell Command Language): 2.8.1 Consequences of Shell Errors For a non-interactive shell, an error condition encountered by a special built-in (see Special Built-In Utilities) or other type of utility shall cause the shell to write a diagnostic message to standard error and exit as shown in the following table: […] In all of the cases shown in the table, an interactive shell shall write a diagnostic message to standard error without exiting. The shell doesn't exit, but it stops evaluation of the startup file(s) instead of simply printing the error and setting $? to 1. Buggy imho. P.S.: thanks for CCing me, i've forgot that.. --steffen
Re: [NEW] ugold(4) driver for Microdia's USB TEMPer variant
Hello, sorry for late reply.
wow, there is many comments...
But anyway, I think that in your case you don't need it. Why don't you
update directly your sensor values when you receive the data in
ugold_intr() instead of copying to a intermediate buffer?
The device always works command (control pipe) - response (interrupt
pipe) cycle. I tried to replace uhidev_get_report() instead of reading
interrupt pipe, but it didn't work. (why?)
If only temperature data (as read temperature data command's response)
comes from interrupt pipe, I think it will be able to remove
intermediate buffer. But, other responses by initialize commands are
also using interrupt pipe. Is there any good idea to distinguish them?
+/*
+ * interrupt pipe is opened but data comes after ugold_attach()
+ * is finished. simply attach sensors here and the device will be
+ * initialized at ugold_refresh().
+ *
+ * at this point, the number of sensors is unknown. setup maximum
+ * sensors here and detach unused sensor later.
+ */
Why don't you initialize the device before opening the interrupt pipe?
ugold_init_device() gets the number of sensors and offset parameter,
they come from interrupt pipe. And I couldn't get any interrupt response
when issuing commands in ugold_attach().
After (exiting) ugold_attach(), I could get interrupt response.
+int
+ugold_issue_cmd(struct ugold_softc *sc, uint8_t *cmd, int len, int delay)
+{
+usb_device_request_t req;
+
+bzero(sc-sc_ibuf, sc-sc_ilen);
+
+req.bmRequestType = UT_WRITE_CLASS_INTERFACE;
+req.bRequest = UR_SET_REPORT;
+USETW(req.wValue, 0x0200);
+USETW(req.wIndex, 0x0001);
+USETW(req.wLength, len);
+if (usbd_do_request(sc-sc_udev, req, cmd))
+return EIO;
I would suggest you to have a look at uhidev_set_report{,_async}() instead of
writing your own version here.
I think this can replace with uhidev_set_report(), I will try rewrite.
+int
+ugold_init_device(struct ugold_softc *sc)
+{
+usb_device_request_t req;
+usbd_status error;
+
+/* send SetReport request to another (Keyboard) interface */
+req.bmRequestType = UT_WRITE_CLASS_INTERFACE;
+req.bRequest = UR_SET_REPORT;
+USETW(req.wValue, 0x0201);
+USETW(req.wIndex, 0x);
+USETW(req.wLength, sizeof(cmd_led_off));
+error = usbd_do_request(sc-sc_udev, req, cmd_led_off);
+if (error)
+return EIO;
Same here, this is likely to be another uhidev_set_report() with a
different interface, no?
Current ugold_attach() ignores keyboard interface, so sc_hdev in
ugold_softc points mouse inerface.
Maybe this SetReport will be able to replace issuing
uhidev_set_report() when detecting keyboard interface in
ugold_attach().
+/* init process */
+if (ugold_issue_cmd(sc, cmd_get_offset, sizeof(cmd_get_offset), 200))
+return EIO;
+
+/* received one interrupt message, it contains offset parameter */
+sc-sc_num_sensors = sc-sc_ibuf[1];
How can you be sure sc_ibuf contains the data you asked for?
Well, sanity check will be required.
Cheers,
--
SASANO Takayoshi u...@mx5.nisiq.net
Re: [NEW] ugold(4) driver for Microdia's USB TEMPer variant
On 2013/05/02 23:57, SASANO Takayoshi wrote:
+int
+ugold_issue_cmd(struct ugold_softc *sc, uint8_t *cmd, int len, int delay)
+{
+ usb_device_request_t req;
+
+ bzero(sc-sc_ibuf, sc-sc_ilen);
+
+ req.bmRequestType = UT_WRITE_CLASS_INTERFACE;
+ req.bRequest = UR_SET_REPORT;
+ USETW(req.wValue, 0x0200);
+ USETW(req.wIndex, 0x0001);
+ USETW(req.wLength, len);
+ if (usbd_do_request(sc-sc_udev, req, cmd))
+ return EIO;
I would suggest you to have a look at uhidev_set_report{,_async}() instead
of
writing your own version here.
I think this can replace with uhidev_set_report(), I will try rewrite.
*if* sc-sc_hdev.sc_report_id is set to 0 then I think this may be
something like: uhidev_set_report(sc-sc_hdev, 2, cmd, len)
the ugold(4) hardware I currently have access to is remote, so I can't
test this easily ;)
if sc_report_id isn't set how we need, then maybe we could at least
call usbd_set_report with hardcoded id.
Re: Test needed: ehci(4) suspend/resume rework
On Tue, Apr 30, 2013 at 09:51:21AM +0200, Martin Pieuchot wrote: On 29/04/13(Mon) 13:25, patrick keshishian wrote: On Sun, Apr 28, 2013 at 03:44:09PM +0200, Martin Pieuchot wrote: Diff below is a rework of the suspend/resume logic in ehci(4). If you're not interested in the full story below, please make sure it doesn't introduce any regression on your machine(s) and, in any case report to me (with your dmesg!) thanks :) Full story: So this diff changes the way we supsend/resume ehci(4) in various way. It started as a simple rewrite of the ehci_reset() function to fix a race while working on the rework of the suspend/resume framework with deraadt@, then later on I discovered that some of the magic in there was wrong or no longer needed. First of all, it removes the logic introduced in r1.46 that places all the ports into suspend mode because it no longer makes sense as we *do* detach/reattach USB devices during a suspend/resume cycle now. Naive question here, but would this be a cause for issue reported here: http://marc.info/?l=openbsd-miscm=136327530312197w=2 Certainly. or at least I'm curious as to why the Wacom tablet does not reattach after a resume. Really? Are you sure it doesn't reattach or is it just not usable in X? It's easy to check, try to suspend/resume in console with the tablet connected, does it show up again in your dmesg? I'm pretty sure it did not, at least at the time when I posted the original message (ref above) to misc@. As noted in that post, the little hey, i'm active LED on the Wacom tablet would not turn on after resume. However, I had some trouble reproducing this condition since your reply. This behavior seems USB port dependent: $ tail -f /var/log/messages May 2 10:51:47 noir /bsd: uhidev0 at uhub2 May 2 10:51:47 noir /bsd: port 1 configuration 1 interface 0 WACOM ET-0405-UV1.1-1 rev 1.00/1.11 addr 3 May 2 10:51:47 noir /bsd: uhidev0: iclass 3/1, 3 report ids May 2 10:51:47 noir /bsd: uhid0 at uhidev0 reportid 2: input=7, output=0, feature=2 May 2 10:51:47 noir /bsd: uhid1 at uhidev0 reportid 3: input=0, output=0, feature=2 May 2 10:52:31 noir apmd: system resumed from APM sleep May 2 10:52:31 noir /bsd: sd1 detached May 2 10:52:32 noir /bsd: scsibus1 detached May 2 10:52:32 noir /bsd: umass0 detached May 2 10:52:32 noir /bsd: umass0 at uhub1 May 2 10:52:32 noir /bsd: port 2 configuration 1 interface 0 Generic USB2.0-CRW rev 2.00/58.88 addr 2 May 2 10:52:32 noir /bsd: umass0: using SCSI over Bulk-Only May 2 10:52:32 noir /bsd: scsibus1 at umass0: 2 targets, initiator 0 May 2 10:52:32 noir /bsd: sd1 at scsibus1 targ 1 lun 0: Generic-, Multi-Card, 1.00 SCSI0 0/direct removable serial.0bda015811417340 May 2 10:52:32 noir /bsd: video0 detached May 2 10:52:32 noir /bsd: uvideo0 detached May 2 10:52:33 noir /bsd: uvideo0 at uhub1 May 2 10:52:34 noir /bsd: port 5 configuration 1 interface 0 Image Processor Integrated Camera rev 2.00/0.29 addr 3 May 2 10:52:34 noir /bsd: video0 at uvideo0 May 2 10:52:34 noir /bsd: uhid0 detached May 2 10:52:34 noir /bsd: uhid1 detached May 2 10:52:34 noir /bsd: uhidev0 detached May 2 10:52:34 noir /bsd: uhidev0 at uhub2 May 2 10:52:34 noir /bsd: port 1 configuration 1 interface 0 WACOM ET-0405-UV1.1-1 rev 1.00/1.11 addr 3 May 2 10:52:34 noir /bsd: uhidev0: iclass 3/1, 3 report ids May 2 10:52:34 noir /bsd: uhid0 at uhidev0 reportid 2: input=7, output=0, feature=2 May 2 10:52:34 noir /bsd: uhid1 at uhidev0 reportid 3: input=0, output=0, feature=2 May 2 10:52:34 noir /bsd: ugen0 detached May 2 10:52:35 noir /bsd: ugen0 at uhub2 May 2 10:52:35 noir /bsd: port 4 Broadcom Corp Broadcom Bluetooth Device rev 2.00/7.48 addr 2 Switching USB ports the Wacom is connected to: May 2 10:52:46 noir /bsd: uhid0 detached May 2 10:52:46 noir /bsd: uhid1 detached May 2 10:52:46 noir /bsd: uhidev0 detached May 2 10:52:50 noir /bsd: uhidev0 at uhub2 May 2 10:52:50 noir /bsd: port 3 configuration 1 interface 0 WACOM ET-0405-UV1.1-1 rev 1.00/1.11 addr 3 May 2 10:52:50 noir /bsd: uhidev0: iclass 3/1, 3 report ids May 2 10:52:50 noir /bsd: uhid0 at uhidev0 reportid 2: input=7, output=0, feature=2 May 2 10:52:50 noir /bsd: uhid1 at uhidev0 reportid 3: input=0, output=0, feature=2 May 2 10:53:32 noir apmd: system resumed from APM sleep May 2 10:53:32 noir /bsd: sd1 detached May 2 10:53:33 noir /bsd: scsibus1 detached May 2 10:53:33 noir /bsd: umass0 detached May 2 10:53:33 noir /bsd: umass0 at uhub1 May 2 10:53:33 noir /bsd: port 2 configuration 1 interface 0 Generic USB2.0-CRW rev 2.00/58.88 addr 2 May 2 10:53:33 noir /bsd: umass0: using SCSI over Bulk-Only May 2 10:53:33 noir /bsd: scsibus1 at umass0: 2 targets, initiator 0 May 2 10:53:33 noir /bsd: sd1 at scsibus1 targ 1 lun 0: Generic-, Multi-Card, 1.00 SCSI0 0/direct removable serial.0bda015811417340 May 2 10:53:33 noir
Re: DPI for pf(4)
On Thu, 2 May 2013, Damien Miller wrote: You've just described bpf, right down to no endless loops and the amount of data it returns. For a little more code that it takes to write one packet parser (basically: loading bpf rules from pf and making the bpf_filter()'s return value available to it) you get everything you described above and more. Actually, you could even make the bpf inspection stateful and bi-directional if you preserved its scratch memory between packets. -d
