linux-kernel - Re: [PATCH] USB: add USB test and measurement class driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 29 Aug 2008 18:41:15 +0200
From:	Marcel Janssen <korgull@...e.nl>
To:	Greg KH <greg@...ah.com>
Cc:	stefan_kopp@...lent.com, stern@...land.harvard.edu,
	oliver@...kum.org, linux-usb@...r.kernel.org, me@...ipebalbi.com,
	linux-kernel@...r.kernel.org,
	Marcel Janssen <marcel.janssen@...esy.nl>
Subject: Re: [PATCH] USB: add USB test and measurement class driver - round 2

On Friday 29 August 2008 16:39:02 Greg KH wrote:
> > The issue with using cat on the shell level is that it uses fread
> > which has the (in this case) ugly behaviour of recalling the driver's
> > read method until the full number of characters requested has been
> > accumulated (or until zero characters are returned, indicating the end
> > of file). With USBTMC instruments, this behavour is bad because the
> > retry will not just return zero characters, it will cause a timeout
> > with the associated error condition in the device. So, to enable the
> > use of echo/cat, I added some fread handling to the driver (which
> > catches the retries). I believe this also has been removed, so I
> > assume cat/fread will not work (?).
>
> I do not know, but we do not do wierd things in the kernel just because
> of broken userspace programs.  This logic should be done in userspace,
> and programs should look at the return value of read() and handle it
> properly.  Otherwise it is a bug.

I don't think this is broken in user space. The problem is that when you issue 
a measurement command it is not known how many bytes it will return. This is 
probably due to ASCII output being very common in  T&M devices instead of raw 
data (int, float etc). The ASCII formatting is done in the device and this 
returns just a string which may or may not be terminated by the term char. 
This is of course not true for all T&M devices, but the majority works this 
way. 

I admit that the above produces a lot of overhead, but it's just a fact that 
T&M devices work this way, including ours for most of their data processing 
(not all though). 

I think the USBTMC spec is quite clear on how it should be implemented on 
messaging level. Basically when you issue  the command "*IDN?" the device 
will process this and return the device ID string. The length of this string 
is set in the TransferSize of the 12 byte header that the device returns. The 
problem when you issue a read command is that the read command does not yet 
know how much data to expect. It should issue the  REQUEST_DEV_DEP_MSG_IN 
first and set the TransferSize value high enough.
In the USBTMC_488 extension you find an example (chapter 3.3.1 page 7) that 
shows the REQUEST_DEV_DEP_MSG_IN TransferSize being set to 64 although the 
actual data being returned from the device is less (only 36 bytes).

What you do see in practice is that when someone would issue a read command 
and asking for less bytes than are available is that the user program may 
handle this as a warning telling the user that he did not request all 
available data. 

Stefan's driver works exactly the way I would expect from a users point of 
view. Whether the implementation can be improved is another issue, but the 
behaviour is correct and compliant with other usbtmc drivers on other 
platforms.

Regards,
Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/