linux-kernel - Re: [v11,1/4] drivers: jtag: Add JTAG core 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:   Sun, 05 Nov 2017 14:32:53 -0800
From:   Chip Bilbrey <chip@...brey.org>
To:     Oleksandr Shamray <oleksandrs@...lanox.com>
Cc:     gregkh@...uxfoundation.org, arnd@...db.de,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        devicetree@...r.kernel.org, openbmc@...ts.ozlabs.org,
        joel@....id.au, jiri@...nulli.us, tklauser@...tanz.ch,
        linux-serial@...r.kernel.org, mec@...ut.net, vadimp@...lanox.com,
        system-sw-low-level@...lanox.com, robh+dt@...nel.org,
        openocd-devel-owner@...ts.sourceforge.net,
        linux-api@...r.kernel.org, davem@...emloft.net, mchehab@...nel.org,
        Jiri Pirko <jiri@...lanox.com>
Subject: Re: [v11,1/4] drivers: jtag: Add JTAG core driver


Oleksandr Shamray writes:
> diff --git a/include/uapi/linux/jtag.h b/include/uapi/linux/jtag.h
> new file mode 100644
> index 0000000..0b25a83
> --- /dev/null
> +++ b/include/uapi/linux/jtag.h
> [...]
> +/**
> + * enum jtag_xfer_mode:
> + *
> + * @JTAG_XFER_HW_MODE: hardware mode transfer
> + * @JTAG_XFER_SW_MODE: software mode transfer
> + */
> +enum jtag_xfer_mode {
> +	JTAG_XFER_HW_MODE,
> +	JTAG_XFER_SW_MODE,
> +};

Is this essentially selecting between bit-bang mode or not?  Is there a
generally applicable reason to select SW mode over HW (or vice versa)?
This sounds like it's tied to device-specific capability which shouldn't
be exposed in a generic user API.

> +/**
> + * struct jtag_xfer - jtag xfer:
> + *
> + * @mode: access mode
> + * @type: transfer type
> + * @direction: xfer direction
> + * @length: xfer bits len
> + * @tdio : xfer data array
> + * @endir: xfer end state
> + *
> + * Structure represents interface to Aspeed JTAG device for jtag sdr xfer
> + * execution.

Probably should remove the reference to Aspeed here.

> +/* ioctl interface */
> +#define __JTAG_IOCTL_MAGIC	0xb2
> +
> +#define JTAG_IOCRUNTEST	_IOW(__JTAG_IOCTL_MAGIC, 0,\
> +			     struct jtag_run_test_idle)
> +#define JTAG_SIOCFREQ	_IOW(__JTAG_IOCTL_MAGIC, 1, unsigned int)
> +#define JTAG_GIOCFREQ	_IOR(__JTAG_IOCTL_MAGIC, 2, unsigned int)
> +#define JTAG_IOCXFER	_IOWR(__JTAG_IOCTL_MAGIC, 3, struct jtag_xfer)
> +#define JTAG_GIOCSTATUS _IOWR(__JTAG_IOCTL_MAGIC, 4, enum jtag_endstate)

I notice the single-open()-per-device lock was dropped by request in an
earlier revision of your patches, but multiple processes trying to drive
a single JTAG master could wreak serious havoc if transactions get
interleaved. Would something like an added JTAG_LOCKCHAIN/UNLOCKCHAIN
ioctl() for exclusive client access be reasonable to prevent this?

-Chip