[PATCHv2 0/7] cec: various improvements

[PATCHv2 0/7] cec: various improvements

[Hans Verkuil]

This series adds a new CEC_MSG_FL_RAW flag to skip all CEC message
checks and transmit the raw message (if called as root).

This is useful when debugging issues with other dubious CEC implementations.

It also relaxes an initiator check for Ping and Image/Text View On
messages when there is no HPD.

Currently the initiator is required to be 0xf, but this is not
actually specified anywhere in the CEC specification, and there are
indications that some displays ignore messages with initiator 0xf.

Regards,

	Hans

Changes since v1:

- Added patch 'cec-ioc-g-mode.rst: be more specific when EPERM is returned'
- CEC_MSG_FL_RAW now needs CAP_SYS_RAWIO instead of CAP_NET_ADMIN.
- Improved the cec-ioc-receive.rst documentation.

Hans Verkuil (7):
  cec: cec_transmit_msg_fh: do sanity checks first
  cec: move check from cec_transmit to cec_transmit_msg_fh
  cec: add CEC_MSG_FL_RAW flag and msg_is_raw helper function
  cec-ioc-receive.rst: document CEC_MSG_FL_RAW
  cec: support CEC_MSG_FL_RAW
  cec: allow any initiator for Ping and Image/Text View On
  cec-ioc-g-mode.rst: be more specific when EPERM is returned

 .../media/uapi/cec/cec-ioc-g-mode.rst         |   3 +-
 .../media/uapi/cec/cec-ioc-receive.rst        |  15 ++-
 drivers/media/cec/cec-adap.c                  | 112 +++++++++++-------
 drivers/media/cec/cec-api.c                   |   8 --
 drivers/media/cec/cec-priv.h                  |   5 +
 include/uapi/linux/cec.h                      |   1 +
 6 files changed, 94 insertions(+), 50 deletions(-)

-- 
2.20.1

[PATCHv2 1/7] cec: cec_transmit_msg_fh: do sanity checks first

[Hans Verkuil]

The code that fills in the CEC_MSG_CDC_MESSAGE physical address
is now done after the sanity checks. It also only does this if the
message length is >= 4 (i.e. there is room for the physical address).

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/cec-adap.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index f1261cc2b6fa..b6102510e203 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -740,11 +740,6 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 	else
 		msg->flags = 0;
 
-	if (msg->len > 1 && msg->msg[1] == CEC_MSG_CDC_MESSAGE) {
-		msg->msg[2] = adap->phys_addr >> 8;
-		msg->msg[3] = adap->phys_addr & 0xff;
-	}
-
 	/* Sanity checks */
 	if (msg->len == 0 || msg->len > CEC_MAX_MSG_SIZE) {
 		dprintk(1, "%s: invalid length %d\n", __func__, msg->len);
@@ -765,6 +760,12 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 		dprintk(1, "%s: can't reply to poll msg\n", __func__);
 		return -EINVAL;
 	}
+
+	if (msg->len >= 4 && msg->msg[1] == CEC_MSG_CDC_MESSAGE) {
+		msg->msg[2] = adap->phys_addr >> 8;
+		msg->msg[3] = adap->phys_addr & 0xff;
+	}
+
 	if (msg->len == 1) {
 		if (cec_msg_destination(msg) == 0xf) {
 			dprintk(1, "%s: invalid poll message\n", __func__);
-- 
2.20.1

[PATCHv2 2/7] cec: move check from cec_transmit to cec_transmit_msg_fh

[Hans Verkuil]

This ensures all the cec_msg checks are done in the same place.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/cec-adap.c | 5 +++++
 drivers/media/cec/cec-api.c  | 8 --------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index b6102510e203..5b9232b6e663 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -761,6 +761,11 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 		return -EINVAL;
 	}
 
+	/* A CDC-Only device can only send CDC messages */
+	if ((adap->log_addrs.flags & CEC_LOG_ADDRS_FL_CDC_ONLY) &&
+	    (msg->len == 1 || msg->msg[1] != CEC_MSG_CDC_MESSAGE))
+		return -EINVAL;
+
 	if (msg->len >= 4 && msg->msg[1] == CEC_MSG_CDC_MESSAGE) {
 		msg->msg[2] = adap->phys_addr >> 8;
 		msg->msg[3] = adap->phys_addr & 0xff;
diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c
index 156a0d76ab2a..12d676484472 100644
--- a/drivers/media/cec/cec-api.c
+++ b/drivers/media/cec/cec-api.c
@@ -198,19 +198,11 @@ static long cec_transmit(struct cec_adapter *adap, struct cec_fh *fh,
 	if (copy_from_user(&msg, parg, sizeof(msg)))
 		return -EFAULT;
 
-	/* A CDC-Only device can only send CDC messages */
-	if ((adap->log_addrs.flags & CEC_LOG_ADDRS_FL_CDC_ONLY) &&
-	    (msg.len == 1 || msg.msg[1] != CEC_MSG_CDC_MESSAGE))
-		return -EINVAL;
-
 	mutex_lock(&adap->lock);
 	if (adap->log_addrs.num_log_addrs == 0)
 		err = -EPERM;
 	else if (adap->is_configuring)
 		err = -ENONET;
-	else if (!adap->is_configured &&
-		 (adap->needs_hpd || msg.msg[0] != 0xf0))
-		err = -ENONET;
 	else if (cec_is_busy(adap, fh))
 		err = -EBUSY;
 	else
-- 
2.20.1

[PATCHv2 3/7] cec: add CEC_MSG_FL_RAW flag and msg_is_raw helper function

[Hans Verkuil]

This adds the userspace API to send raw unchecked CEC messages.
This will require root permissions.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/cec-priv.h | 5 +++++
 include/uapi/linux/cec.h     | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/media/cec/cec-priv.h b/drivers/media/cec/cec-priv.h
index 804e38f849c7..7bdf855aaecd 100644
--- a/drivers/media/cec/cec-priv.h
+++ b/drivers/media/cec/cec-priv.h
@@ -20,6 +20,11 @@
 /* devnode to cec_adapter */
 #define to_cec_adapter(node) container_of(node, struct cec_adapter, devnode)
 
+static inline bool msg_is_raw(const struct cec_msg *msg)
+{
+	return msg->flags & CEC_MSG_FL_RAW;
+}
+
 /* cec-core.c */
 extern int cec_debug;
 int cec_get_device(struct cec_devnode *devnode);
diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
index 3094af68b6e7..5704fa0292b5 100644
--- a/include/uapi/linux/cec.h
+++ b/include/uapi/linux/cec.h
@@ -144,6 +144,7 @@ static inline void cec_msg_set_reply_to(struct cec_msg *msg,
 
 /* cec_msg flags field */
 #define CEC_MSG_FL_REPLY_TO_FOLLOWERS	(1 << 0)
+#define CEC_MSG_FL_RAW			(1 << 1)
 
 /* cec_msg tx/rx_status field */
 #define CEC_TX_STATUS_OK		(1 << 0)
-- 
2.20.1

[PATCHv2 4/7] cec-ioc-receive.rst: document CEC_MSG_FL_RAW

[Hans Verkuil]

Document this new cec_msg flag.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/media/uapi/cec/cec-ioc-receive.rst | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/cec/cec-ioc-receive.rst b/Documentation/media/uapi/cec/cec-ioc-receive.rst
index c3a685ff05cb..4137903d672e 100644
--- a/Documentation/media/uapi/cec/cec-ioc-receive.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-receive.rst
@@ -223,6 +223,18 @@ View On' messages from initiator 0xf ('Unregistered') to destination 0 ('TV').
 	result of the :ref:`ioctl CEC_TRANSMIT <CEC_TRANSMIT>`, and once via
 	:ref:`ioctl CEC_RECEIVE <CEC_RECEIVE>`.
 
+    * .. _`CEC-MSG-FL-RAW`:
+
+      - ``CEC_MSG_FL_RAW``
+      - 2
+      - Normally CEC messages are validated before transmitting them. If this
+        flag is set when :ref:`ioctl CEC_TRANSMIT <CEC_TRANSMIT>` is called,
+	then no validation takes place and the message is transmitted as-is.
+	This is useful when debugging CEC issues.
+	This flag is only allowed if the process has the ``CAP_SYS_RAWIO``
+	capability. If that is not set, then the ``EPERM`` error code is
+	returned.
+
 
 .. tabularcolumns:: |p{5.6cm}|p{0.9cm}|p{11.0cm}|
 
@@ -358,7 +370,8 @@ ENOTTY
 
 EPERM
     The CEC adapter is not configured, i.e. :ref:`ioctl CEC_ADAP_S_LOG_ADDRS <CEC_ADAP_S_LOG_ADDRS>`
-    has never been called.
+    has never been called, or ``CEC_MSG_FL_RAW`` was used from a process that
+    did not have the ``CAP_SYS_RAWIO`` capability.
 
 ENONET
     The CEC adapter is not configured, i.e. :ref:`ioctl CEC_ADAP_S_LOG_ADDRS <CEC_ADAP_S_LOG_ADDRS>`
-- 
2.20.1

[PATCHv2 5/7] cec: support CEC_MSG_FL_RAW

[Hans Verkuil]

If this flag is set, then check for root permissions and skip
all message checks expect for the core checks (i.e. validate the
length etc.).

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/cec-adap.c | 107 ++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 45 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 5b9232b6e663..9a1ec9299aca 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -720,6 +720,7 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 			struct cec_fh *fh, bool block)
 {
 	struct cec_data *data;
+	bool is_raw = msg_is_raw(msg);
 
 	msg->rx_ts = 0;
 	msg->tx_ts = 0;
@@ -735,10 +736,10 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 		/* Make sure the timeout isn't 0. */
 		msg->timeout = 1000;
 	}
-	if (msg->timeout)
-		msg->flags &= CEC_MSG_FL_REPLY_TO_FOLLOWERS;
-	else
-		msg->flags = 0;
+	msg->flags &= CEC_MSG_FL_REPLY_TO_FOLLOWERS | CEC_MSG_FL_RAW;
+
+	if (!msg->timeout)
+		msg->flags &= ~CEC_MSG_FL_REPLY_TO_FOLLOWERS;
 
 	/* Sanity checks */
 	if (msg->len == 0 || msg->len > CEC_MAX_MSG_SIZE) {
@@ -761,54 +762,70 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 		return -EINVAL;
 	}
 
-	/* A CDC-Only device can only send CDC messages */
-	if ((adap->log_addrs.flags & CEC_LOG_ADDRS_FL_CDC_ONLY) &&
-	    (msg->len == 1 || msg->msg[1] != CEC_MSG_CDC_MESSAGE))
-		return -EINVAL;
+	if (is_raw) {
+		if (!capable(CAP_SYS_RAWIO))
+			return -EPERM;
+	} else {
+		/* A CDC-Only device can only send CDC messages */
+		if ((adap->log_addrs.flags & CEC_LOG_ADDRS_FL_CDC_ONLY) &&
+		    (msg->len == 1 || msg->msg[1] != CEC_MSG_CDC_MESSAGE)) {
+			dprintk(1, "%s: not a CDC message\n", __func__);
+			return -EINVAL;
+		}
 
-	if (msg->len >= 4 && msg->msg[1] == CEC_MSG_CDC_MESSAGE) {
-		msg->msg[2] = adap->phys_addr >> 8;
-		msg->msg[3] = adap->phys_addr & 0xff;
-	}
+		if (msg->len >= 4 && msg->msg[1] == CEC_MSG_CDC_MESSAGE) {
+			msg->msg[2] = adap->phys_addr >> 8;
+			msg->msg[3] = adap->phys_addr & 0xff;
+		}
 
-	if (msg->len == 1) {
-		if (cec_msg_destination(msg) == 0xf) {
-			dprintk(1, "%s: invalid poll message\n", __func__);
+		if (msg->len == 1) {
+			if (cec_msg_destination(msg) == 0xf) {
+				dprintk(1, "%s: invalid poll message\n",
+					__func__);
+				return -EINVAL;
+			}
+			if (cec_has_log_addr(adap, cec_msg_destination(msg))) {
+				/*
+				 * If the destination is a logical address our
+				 * adapter has already claimed, then just NACK
+				 * this. It depends on the hardware what it will
+				 * do with a POLL to itself (some OK this), so
+				 * it is just as easy to handle it here so the
+				 * behavior will be consistent.
+				 */
+				msg->tx_ts = ktime_get_ns();
+				msg->tx_status = CEC_TX_STATUS_NACK |
+					CEC_TX_STATUS_MAX_RETRIES;
+				msg->tx_nack_cnt = 1;
+				msg->sequence = ++adap->sequence;
+				if (!msg->sequence)
+					msg->sequence = ++adap->sequence;
+				return 0;
+			}
+		}
+		if (msg->len > 1 && !cec_msg_is_broadcast(msg) &&
+		    cec_has_log_addr(adap, cec_msg_destination(msg))) {
+			dprintk(1, "%s: destination is the adapter itself\n",
+				__func__);
 			return -EINVAL;
 		}
-		if (cec_has_log_addr(adap, cec_msg_destination(msg))) {
-			/*
-			 * If the destination is a logical address our adapter
-			 * has already claimed, then just NACK this.
-			 * It depends on the hardware what it will do with a
-			 * POLL to itself (some OK this), so it is just as
-			 * easy to handle it here so the behavior will be
-			 * consistent.
-			 */
-			msg->tx_ts = ktime_get_ns();
-			msg->tx_status = CEC_TX_STATUS_NACK |
-					 CEC_TX_STATUS_MAX_RETRIES;
-			msg->tx_nack_cnt = 1;
-			msg->sequence = ++adap->sequence;
-			if (!msg->sequence)
-				msg->sequence = ++adap->sequence;
-			return 0;
+		if (msg->len > 1 && adap->is_configured &&
+		    !cec_has_log_addr(adap, cec_msg_initiator(msg))) {
+			dprintk(1, "%s: initiator has unknown logical address %d\n",
+				__func__, cec_msg_initiator(msg));
+			return -EINVAL;
+		}
+		if (!adap->is_configured && !adap->is_configuring &&
+		    msg->msg[0] != 0xf0) {
+			dprintk(1, "%s: adapter is unconfigured\n", __func__);
+			return -ENONET;
 		}
 	}
-	if (msg->len > 1 && !cec_msg_is_broadcast(msg) &&
-	    cec_has_log_addr(adap, cec_msg_destination(msg))) {
-		dprintk(1, "%s: destination is the adapter itself\n", __func__);
-		return -EINVAL;
-	}
-	if (msg->len > 1 && adap->is_configured &&
-	    !cec_has_log_addr(adap, cec_msg_initiator(msg))) {
-		dprintk(1, "%s: initiator has unknown logical address %d\n",
-			__func__, cec_msg_initiator(msg));
-		return -EINVAL;
-	}
+
 	if (!adap->is_configured && !adap->is_configuring) {
-		if (adap->needs_hpd || msg->msg[0] != 0xf0) {
-			dprintk(1, "%s: adapter is unconfigured\n", __func__);
+		if (adap->needs_hpd) {
+			dprintk(1, "%s: adapter is unconfigured and needs HPD\n",
+				__func__);
 			return -ENONET;
 		}
 		if (msg->reply) {
-- 
2.20.1

[PATCHv2 6/7] cec: allow any initiator for Ping and Image/Text View On

[Hans Verkuil]

Some displays pull down the HPD when in standby, but CEC is still
active and the display can be woken up by sending an Image View On
or Text View On CEC command. The CEC specification doesn't tell you
what the initiator should be for such a command (without a HPD it's
unclear if the CEC adapter can claim a logical address).

This patch allows any initiator value when there is no HPD for the
Image/Text View On commands and for the Ping command.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/cec/cec-adap.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 9a1ec9299aca..5827d8c3742a 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -809,14 +809,23 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 				__func__);
 			return -EINVAL;
 		}
-		if (msg->len > 1 && adap->is_configured &&
+		if (adap->is_configured &&
 		    !cec_has_log_addr(adap, cec_msg_initiator(msg))) {
 			dprintk(1, "%s: initiator has unknown logical address %d\n",
 				__func__, cec_msg_initiator(msg));
 			return -EINVAL;
 		}
+		/*
+		 * Special case: allow Ping and IMAGE/TEXT_VIEW_ON to be
+		 * transmitted to a TV, even if the adapter is unconfigured.
+		 * This makes it possible to detect or wake up displays that
+		 * pull down the HPD when in standby.
+		 */
 		if (!adap->is_configured && !adap->is_configuring &&
-		    msg->msg[0] != 0xf0) {
+		    (msg->len > 2 ||
+		     cec_msg_destination(msg) != CEC_LOG_ADDR_TV ||
+		     (msg->len == 2 && msg->msg[1] != CEC_MSG_IMAGE_VIEW_ON &&
+		      msg->msg[1] != CEC_MSG_TEXT_VIEW_ON))) {
 			dprintk(1, "%s: adapter is unconfigured\n", __func__);
 			return -ENONET;
 		}
-- 
2.20.1

[PATCHv2 7/7] cec-ioc-g-mode.rst: be more specific when EPERM is returned

[Hans Verkuil]

Document which capability is required, rather than just saying
that "root permissions" are required.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 Documentation/media/uapi/cec/cec-ioc-g-mode.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/media/uapi/cec/cec-ioc-g-mode.rst b/Documentation/media/uapi/cec/cec-ioc-g-mode.rst
index c53bb5f73f0d..d0902f356d65 100644
--- a/Documentation/media/uapi/cec/cec-ioc-g-mode.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-g-mode.rst
@@ -294,7 +294,8 @@ EINVAL
     The requested mode is invalid.
 
 EPERM
-    Monitor mode is requested without having root permissions
+    Monitor mode is requested, but the process does have the ``CAP_NET_ADMIN``
+    capability.
 
 EBUSY
     Someone else is already an exclusive follower or initiator.
-- 
2.20.1