[PATCH] greybus: Avoid fake flexible array for response data

[PATCH] greybus: Avoid fake flexible array for response data

[Kees Cook]

FORTIFY_SOURCE has been ignoring 0-sized destinations while the kernel
code base has been converted to flexible arrays. In order to enforce
the 0-sized destinations (e.g. with __counted_by), the remaining 0-sized
destinations need to be handled. Instead of converting an empty struct
into using a flexible array, just directly use a pointer without any
additional indirection. Remove struct gb_bootrom_get_firmware_response
and struct gb_fw_download_fetch_firmware_response.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: Viresh Kumar <vireshk@kernel.org>
Cc: Johan Hovold <johan@kernel.org>
Cc: Alex Elder <elder@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Gustavo A. R. Silva <gustavo@embeddedor.com>
Cc: greybus-dev@lists.linaro.org
Cc: linux-staging@lists.linux.dev
---
 drivers/staging/greybus/bootrom.c         | 7 +++----
 drivers/staging/greybus/fw-download.c     | 7 +++----
 include/linux/greybus/greybus_protocols.h | 8 --------
 3 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
index 79581457c4af..76ad5f289072 100644
--- a/drivers/staging/greybus/bootrom.c
+++ b/drivers/staging/greybus/bootrom.c
@@ -243,10 +243,10 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
 	struct gb_bootrom *bootrom = gb_connection_get_data(op->connection);
 	const struct firmware *fw;
 	struct gb_bootrom_get_firmware_request *firmware_request;
-	struct gb_bootrom_get_firmware_response *firmware_response;
 	struct device *dev = &op->connection->bundle->dev;
 	unsigned int offset, size;
 	enum next_request_type next_request;
+	u8 *firmware_response;
 	int ret = 0;
 
 	/* Disable timeouts */
@@ -280,15 +280,14 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
 		goto unlock;
 	}
 
-	if (!gb_operation_response_alloc(op, sizeof(*firmware_response) + size,
-					 GFP_KERNEL)) {
+	if (!gb_operation_response_alloc(op, size, GFP_KERNEL)) {
 		dev_err(dev, "%s: error allocating response\n", __func__);
 		ret = -ENOMEM;
 		goto unlock;
 	}
 
 	firmware_response = op->response->payload;
-	memcpy(firmware_response->data, fw->data + offset, size);
+	memcpy(firmware_response, fw->data + offset, size);
 
 	dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n",
 		offset, size);
diff --git a/drivers/staging/greybus/fw-download.c b/drivers/staging/greybus/fw-download.c
index 543692c567f9..2d73bb729aa2 100644
--- a/drivers/staging/greybus/fw-download.c
+++ b/drivers/staging/greybus/fw-download.c
@@ -271,11 +271,11 @@ static int fw_download_fetch_firmware(struct gb_operation *op)
 	struct gb_connection *connection = op->connection;
 	struct fw_download *fw_download = gb_connection_get_data(connection);
 	struct gb_fw_download_fetch_firmware_request *request;
-	struct gb_fw_download_fetch_firmware_response *response;
 	struct fw_request *fw_req;
 	const struct firmware *fw;
 	unsigned int offset, size;
 	u8 firmware_id;
+	u8 *response;
 	int ret = 0;
 
 	if (op->request->payload_size != sizeof(*request)) {
@@ -325,8 +325,7 @@ static int fw_download_fetch_firmware(struct gb_operation *op)
 		goto put_fw;
 	}
 
-	if (!gb_operation_response_alloc(op, sizeof(*response) + size,
-					 GFP_KERNEL)) {
+	if (!gb_operation_response_alloc(op, size, GFP_KERNEL)) {
 		dev_err(fw_download->parent,
 			"error allocating fetch firmware response\n");
 		ret = -ENOMEM;
@@ -334,7 +333,7 @@ static int fw_download_fetch_firmware(struct gb_operation *op)
 	}
 
 	response = op->response->payload;
-	memcpy(response->data, fw->data + offset, size);
+	memcpy(response, fw->data + offset, size);
 
 	dev_dbg(fw_download->parent,
 		"responding with firmware (offs = %u, size = %u)\n", offset,
diff --git a/include/linux/greybus/greybus_protocols.h b/include/linux/greybus/greybus_protocols.h
index aeb8f9243545..603acdcc0c6b 100644
--- a/include/linux/greybus/greybus_protocols.h
+++ b/include/linux/greybus/greybus_protocols.h
@@ -232,10 +232,6 @@ struct gb_fw_download_fetch_firmware_request {
 	__le32			size;
 } __packed;
 
-struct gb_fw_download_fetch_firmware_response {
-	__u8			data[0];
-} __packed;
-
 /* firmware download release firmware request */
 struct gb_fw_download_release_firmware_request {
 	__u8			firmware_id;
@@ -414,10 +410,6 @@ struct gb_bootrom_get_firmware_request {
 	__le32			size;
 } __packed;
 
-struct gb_bootrom_get_firmware_response {
-	__u8			data[0];
-} __packed;
-
 /* Bootrom protocol Ready to boot request */
 struct gb_bootrom_ready_to_boot_request {
 	__u8			status;
-- 
2.34.1

Re: [PATCH] greybus: Avoid fake flexible array for response data

[Alex Elder]

On 2/16/24 5:28 PM, Kees Cook wrote:
> FORTIFY_SOURCE has been ignoring 0-sized destinations while the kernel
> code base has been converted to flexible arrays. In order to enforce
> the 0-sized destinations (e.g. with __counted_by), the remaining 0-sized
> destinations need to be handled. Instead of converting an empty struct
> into using a flexible array, just directly use a pointer without any
> additional indirection. Remove struct gb_bootrom_get_firmware_response
> and struct gb_fw_download_fetch_firmware_response.

The only down side I see is that it sort of disrupts a pattern
used on Greybus request handlers (and the response structure definitions).

I think a one-line comment in place of each of these two
definitions would be helpful, something like:
/* gb_fw_download_fetch_firmware_response contains no data */

And then add a similar comment above the calls to
gb_operation_response_alloc().

Otherwise this looks good.

Reviewed-by: Alex Elder <elder@linaro.org>

> 
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> Cc: Viresh Kumar <vireshk@kernel.org>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Alex Elder <elder@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Cc: greybus-dev@lists.linaro.org
> Cc: linux-staging@lists.linux.dev
> ---
>   drivers/staging/greybus/bootrom.c         | 7 +++----
>   drivers/staging/greybus/fw-download.c     | 7 +++----
>   include/linux/greybus/greybus_protocols.h | 8 --------
>   3 files changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/greybus/bootrom.c b/drivers/staging/greybus/bootrom.c
> index 79581457c4af..76ad5f289072 100644
> --- a/drivers/staging/greybus/bootrom.c
> +++ b/drivers/staging/greybus/bootrom.c
> @@ -243,10 +243,10 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
>   	struct gb_bootrom *bootrom = gb_connection_get_data(op->connection);
>   	const struct firmware *fw;
>   	struct gb_bootrom_get_firmware_request *firmware_request;
> -	struct gb_bootrom_get_firmware_response *firmware_response;
>   	struct device *dev = &op->connection->bundle->dev;
>   	unsigned int offset, size;
>   	enum next_request_type next_request;
> +	u8 *firmware_response;
>   	int ret = 0;
>   
>   	/* Disable timeouts */
> @@ -280,15 +280,14 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
>   		goto unlock;
>   	}
>   
> -	if (!gb_operation_response_alloc(op, sizeof(*firmware_response) + size,
> -					 GFP_KERNEL)) {
> +	if (!gb_operation_response_alloc(op, size, GFP_KERNEL)) {
>   		dev_err(dev, "%s: error allocating response\n", __func__);
>   		ret = -ENOMEM;
>   		goto unlock;
>   	}
>   
>   	firmware_response = op->response->payload;
> -	memcpy(firmware_response->data, fw->data + offset, size);
> +	memcpy(firmware_response, fw->data + offset, size);
>   
>   	dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n",
>   		offset, size);
> diff --git a/drivers/staging/greybus/fw-download.c b/drivers/staging/greybus/fw-download.c
> index 543692c567f9..2d73bb729aa2 100644
> --- a/drivers/staging/greybus/fw-download.c
> +++ b/drivers/staging/greybus/fw-download.c
> @@ -271,11 +271,11 @@ static int fw_download_fetch_firmware(struct gb_operation *op)
>   	struct gb_connection *connection = op->connection;
>   	struct fw_download *fw_download = gb_connection_get_data(connection);
>   	struct gb_fw_download_fetch_firmware_request *request;
> -	struct gb_fw_download_fetch_firmware_response *response;
>   	struct fw_request *fw_req;
>   	const struct firmware *fw;
>   	unsigned int offset, size;
>   	u8 firmware_id;
> +	u8 *response;
>   	int ret = 0;
>   
>   	if (op->request->payload_size != sizeof(*request)) {
> @@ -325,8 +325,7 @@ static int fw_download_fetch_firmware(struct gb_operation *op)
>   		goto put_fw;
>   	}
>   
> -	if (!gb_operation_response_alloc(op, sizeof(*response) + size,
> -					 GFP_KERNEL)) {
> +	if (!gb_operation_response_alloc(op, size, GFP_KERNEL)) {
>   		dev_err(fw_download->parent,
>   			"error allocating fetch firmware response\n");
>   		ret = -ENOMEM;
> @@ -334,7 +333,7 @@ static int fw_download_fetch_firmware(struct gb_operation *op)
>   	}
>   
>   	response = op->response->payload;
> -	memcpy(response->data, fw->data + offset, size);
> +	memcpy(response, fw->data + offset, size);
>   
>   	dev_dbg(fw_download->parent,
>   		"responding with firmware (offs = %u, size = %u)\n", offset,
> diff --git a/include/linux/greybus/greybus_protocols.h b/include/linux/greybus/greybus_protocols.h
> index aeb8f9243545..603acdcc0c6b 100644
> --- a/include/linux/greybus/greybus_protocols.h
> +++ b/include/linux/greybus/greybus_protocols.h
> @@ -232,10 +232,6 @@ struct gb_fw_download_fetch_firmware_request {
>   	__le32			size;
>   } __packed;
>   
> -struct gb_fw_download_fetch_firmware_response {
> -	__u8			data[0];
> -} __packed;
> -
>   /* firmware download release firmware request */
>   struct gb_fw_download_release_firmware_request {
>   	__u8			firmware_id;
> @@ -414,10 +410,6 @@ struct gb_bootrom_get_firmware_request {
>   	__le32			size;
>   } __packed;
>   
> -struct gb_bootrom_get_firmware_response {
> -	__u8			data[0];
> -} __packed;
> -
>   /* Bootrom protocol Ready to boot request */
>   struct gb_bootrom_ready_to_boot_request {
>   	__u8			status;

Re: [PATCH] greybus: Avoid fake flexible array for response data

[Kees Cook]

On Sat, Feb 17, 2024 at 02:17:33PM -0600, Alex Elder wrote:
> On 2/16/24 5:28 PM, Kees Cook wrote:
> > FORTIFY_SOURCE has been ignoring 0-sized destinations while the kernel
> > code base has been converted to flexible arrays. In order to enforce
> > the 0-sized destinations (e.g. with __counted_by), the remaining 0-sized
> > destinations need to be handled. Instead of converting an empty struct
> > into using a flexible array, just directly use a pointer without any
> > additional indirection. Remove struct gb_bootrom_get_firmware_response
> > and struct gb_fw_download_fetch_firmware_response.
> 
> The only down side I see is that it sort of disrupts a pattern
> used on Greybus request handlers (and the response structure definitions).
> 
> I think a one-line comment in place of each of these two
> definitions would be helpful, something like:
> /* gb_fw_download_fetch_firmware_response contains no data */

Er, maybe this should be "no other data" ? Do you want a v2 of this
patch?

> And then add a similar comment above the calls to
> gb_operation_response_alloc().
> 
> Otherwise this looks good.
> 
> Reviewed-by: Alex Elder <elder@linaro.org>

Thanks!

-- 
Kees Cook

Re: [PATCH] greybus: Avoid fake flexible array for response data

[Alex Elder]

On 2/17/24 3:58 PM, Kees Cook wrote:
> On Sat, Feb 17, 2024 at 02:17:33PM -0600, Alex Elder wrote:
>> On 2/16/24 5:28 PM, Kees Cook wrote:
>>> FORTIFY_SOURCE has been ignoring 0-sized destinations while the kernel
>>> code base has been converted to flexible arrays. In order to enforce
>>> the 0-sized destinations (e.g. with __counted_by), the remaining 0-sized
>>> destinations need to be handled. Instead of converting an empty struct
>>> into using a flexible array, just directly use a pointer without any
>>> additional indirection. Remove struct gb_bootrom_get_firmware_response
>>> and struct gb_fw_download_fetch_firmware_response.
>>
>> The only down side I see is that it sort of disrupts a pattern
>> used on Greybus request handlers (and the response structure definitions).
>>
>> I think a one-line comment in place of each of these two
>> definitions would be helpful, something like:
>> /* gb_fw_download_fetch_firmware_response contains no data */
> 
> Er, maybe this should be "no other data" ? Do you want a v2 of this
> patch?


Sending v2 is probably best, because I'd like to see these
comments.  Greg could fix it up himself but he probably wants
to pull it from the list

And yes, "no other data" is fine, or maybe "no payload"
or "has an empty payload".  Any of those is better than
nothing; you choose.

Thank you.

					-Alex

> 
>> And then add a similar comment above the calls to
>> gb_operation_response_alloc().
>>
>> Otherwise this looks good.
>>
>> Reviewed-by: Alex Elder <elder@linaro.org>
> 
> Thanks!
> 

[PATCH] greybus: Avoid fake flexible array for response data

[clingfei]

As https://lore.kernel.org/all/20240304211940.it.083-kees@kernel.org/
pointed out, to enforce the 0-sized destinations, the remaining 0-sized
destinations need to be handled. Thus the struct
gb_control_get_manifest_response and struct gb_i2c_transfer_response
are removed.

Signed-off-by: clingfei <clf700383@gmail.com>
---
 drivers/staging/greybus/i2c.c             | 9 ++++-----
 include/linux/greybus/greybus_protocols.h | 9 ---------
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/greybus/i2c.c b/drivers/staging/greybus/i2c.c
index 14f1ff6d448c..2857c2834206 100644
--- a/drivers/staging/greybus/i2c.c
+++ b/drivers/staging/greybus/i2c.c
@@ -144,15 +144,14 @@ gb_i2c_operation_create(struct gb_connection *connection,
 }
 
 static void gb_i2c_decode_response(struct i2c_msg *msgs, u32 msg_count,
-				   struct gb_i2c_transfer_response *response)
+				   u8 *data)
 {
 	struct i2c_msg *msg = msgs;
-	u8 *data;
 	u32 i;
 
-	if (!response)
+	if (!data)
 		return;
-	data = response->data;
+
 	for (i = 0; i < msg_count; i++) {
 		if (msg->flags & I2C_M_RD) {
 			memcpy(msg->buf, data, msg->len);
@@ -188,7 +187,7 @@ static int gb_i2c_transfer_operation(struct gb_i2c_device *gb_i2c_dev,
 
 	ret = gb_operation_request_send_sync(operation);
 	if (!ret) {
-		struct gb_i2c_transfer_response *response;
+		u8 *response;
 
 		response = operation->response->payload;
 		gb_i2c_decode_response(msgs, msg_count, response);
diff --git a/include/linux/greybus/greybus_protocols.h b/include/linux/greybus/greybus_protocols.h
index 820134b0105c..14395f9300d6 100644
--- a/include/linux/greybus/greybus_protocols.h
+++ b/include/linux/greybus/greybus_protocols.h
@@ -110,11 +110,6 @@ struct gb_control_get_manifest_size_response {
 	__le16			size;
 } __packed;
 
-/* Control protocol manifest get request has no payload */
-struct gb_control_get_manifest_response {
-	__u8			data[0];
-} __packed;
-
 /* Control protocol [dis]connected request */
 struct gb_control_connected_request {
 	__le16			cport_id;
@@ -678,10 +673,6 @@ struct gb_i2c_transfer_request {
 	__le16				op_count;
 	struct gb_i2c_transfer_op	ops[];		/* op_count of these */
 } __packed;
-struct gb_i2c_transfer_response {
-	__u8				data[0];	/* inbound data */
-} __packed;
-
 
 /* GPIO */
 
-- 
2.34.1

Re: [PATCH] greybus: Avoid fake flexible array for response data

[Dan Carpenter]

On Mon, May 26, 2025 at 07:06:54PM +0800, clingfei wrote:
> As https://lore.kernel.org/all/20240304211940.it.083-kees@kernel.org/

I don't want to have to read a link to understand the commit message.

Does this change really affect anything in terms of "enforce the 0-sized
destinations" rule?  I don't think this is a destination.  I think Kees
enabled the checking he wanted.  You should probably CC him since we're
refering to his email.

> pointed out, to enforce the 0-sized destinations, the remaining 0-sized
> destinations need to be handled. Thus the struct
> gb_control_get_manifest_response and struct gb_i2c_transfer_response
> are removed.

Here is a better commit message;

"We want to get rid of zero size arrays and use flexible arrays instead.
However, in this case the struct is just one flexible array of u8 which
adds no value.  Just use a char pointer instead."

I would have ignored it, probably but actually the
gb_control_get_manifest_response struct is not used so put that in a
separate commit.  That's a simpler commit to review.

"The gb_control_get_manifest_response struct is not used.  Delete it."

> 
> Signed-off-by: clingfei <clf700383@gmail.com>
> ---
>  drivers/staging/greybus/i2c.c             | 9 ++++-----
>  include/linux/greybus/greybus_protocols.h | 9 ---------
>  2 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/greybus/i2c.c b/drivers/staging/greybus/i2c.c
> index 14f1ff6d448c..2857c2834206 100644
> --- a/drivers/staging/greybus/i2c.c
> +++ b/drivers/staging/greybus/i2c.c
> @@ -144,15 +144,14 @@ gb_i2c_operation_create(struct gb_connection *connection,
>  }
>  
>  static void gb_i2c_decode_response(struct i2c_msg *msgs, u32 msg_count,
> -				   struct gb_i2c_transfer_response *response)
> +				   u8 *data)
>  {
>  	struct i2c_msg *msg = msgs;
> -	u8 *data;
>  	u32 i;
>  
> -	if (!response)
> +	if (!data)
>  		return;
> -	data = response->data;
> +
>  	for (i = 0; i < msg_count; i++) {
>  		if (msg->flags & I2C_M_RD) {
>  			memcpy(msg->buf, data, msg->len);
> @@ -188,7 +187,7 @@ static int gb_i2c_transfer_operation(struct gb_i2c_device *gb_i2c_dev,
>  
>  	ret = gb_operation_request_send_sync(operation);
>  	if (!ret) {
> -		struct gb_i2c_transfer_response *response;
> +		u8 *response;
>  
>  		response = operation->response->payload;
>  		gb_i2c_decode_response(msgs, msg_count, response);

I like when parameters are called the same thing on both sides.  The
name "response" adds no value.  Instead get rid of that variable and
pass operation->response->payload directly.

		gb_i2c_decode_response(msgs, msg_count,
				       operation->response->payload);

regards,
dan carpenter

Re: [PATCH] greybus: Avoid fake flexible array for response data

[Kees Cook]

On Mon, May 26, 2025 at 02:37:13PM +0300, Dan Carpenter wrote:
> On Mon, May 26, 2025 at 07:06:54PM +0800, clingfei wrote:
> > As https://lore.kernel.org/all/20240304211940.it.083-kees@kernel.org/
> 
> I don't want to have to read a link to understand the commit message.
> 
> Does this change really affect anything in terms of "enforce the 0-sized
> destinations" rule?  I don't think this is a destination.  I think Kees
> enabled the checking he wanted.  You should probably CC him since we're
> refering to his email.

FWIW, the above patch became commit 7ba59ac7da2a ("greybus: Avoid fake
flexible array for response data").

> > pointed out, to enforce the 0-sized destinations, the remaining 0-sized
> > destinations need to be handled. Thus the struct
> > gb_control_get_manifest_response and struct gb_i2c_transfer_response
> > are removed.
> 
> Here is a better commit message;
> 
> "We want to get rid of zero size arrays and use flexible arrays instead.
> However, in this case the struct is just one flexible array of u8 which
> adds no value.  Just use a char pointer instead."
> 
> I would have ignored it, probably but actually the
> gb_control_get_manifest_response struct is not used so put that in a
> separate commit.  That's a simpler commit to review.
> 
> "The gb_control_get_manifest_response struct is not used.  Delete it."

The patch content itself looks mechanically correct. I think Dan's style
suggestions would be good to see. Can you send a v2?

-Kees

> 
> > 
> > Signed-off-by: clingfei <clf700383@gmail.com>
> > ---
> >  drivers/staging/greybus/i2c.c             | 9 ++++-----
> >  include/linux/greybus/greybus_protocols.h | 9 ---------
> >  2 files changed, 4 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/i2c.c b/drivers/staging/greybus/i2c.c
> > index 14f1ff6d448c..2857c2834206 100644
> > --- a/drivers/staging/greybus/i2c.c
> > +++ b/drivers/staging/greybus/i2c.c
> > @@ -144,15 +144,14 @@ gb_i2c_operation_create(struct gb_connection *connection,
> >  }
> >  
> >  static void gb_i2c_decode_response(struct i2c_msg *msgs, u32 msg_count,
> > -				   struct gb_i2c_transfer_response *response)
> > +				   u8 *data)
> >  {
> >  	struct i2c_msg *msg = msgs;
> > -	u8 *data;
> >  	u32 i;
> >  
> > -	if (!response)
> > +	if (!data)
> >  		return;
> > -	data = response->data;
> > +
> >  	for (i = 0; i < msg_count; i++) {
> >  		if (msg->flags & I2C_M_RD) {
> >  			memcpy(msg->buf, data, msg->len);
> > @@ -188,7 +187,7 @@ static int gb_i2c_transfer_operation(struct gb_i2c_device *gb_i2c_dev,
> >  
> >  	ret = gb_operation_request_send_sync(operation);
> >  	if (!ret) {
> > -		struct gb_i2c_transfer_response *response;
> > +		u8 *response;
> >  
> >  		response = operation->response->payload;
> >  		gb_i2c_decode_response(msgs, msg_count, response);
> 
> I like when parameters are called the same thing on both sides.  The
> name "response" adds no value.  Instead get rid of that variable and
> pass operation->response->payload directly.
> 
> 		gb_i2c_decode_response(msgs, msg_count,
> 				       operation->response->payload);
> 
> regards,
> dan carpenter
> 

-- 
Kees Cook

Re: [PATCH] greybus: Avoid fake flexible array for response data

[Greg KH]

On Mon, May 26, 2025 at 07:06:54PM +0800, clingfei wrote:
> diff --git a/include/linux/greybus/greybus_protocols.h b/include/linux/greybus/greybus_protocols.h
> index 820134b0105c..14395f9300d6 100644
> --- a/include/linux/greybus/greybus_protocols.h
> +++ b/include/linux/greybus/greybus_protocols.h
> @@ -110,11 +110,6 @@ struct gb_control_get_manifest_size_response {
>  	__le16			size;
>  } __packed;
>  
> -/* Control protocol manifest get request has no payload */
> -struct gb_control_get_manifest_response {
> -	__u8			data[0];
> -} __packed;
> -
>  /* Control protocol [dis]connected request */
>  struct gb_control_connected_request {
>  	__le16			cport_id;
> @@ -678,10 +673,6 @@ struct gb_i2c_transfer_request {
>  	__le16				op_count;
>  	struct gb_i2c_transfer_op	ops[];		/* op_count of these */
>  } __packed;
> -struct gb_i2c_transfer_response {
> -	__u8				data[0];	/* inbound data */
> -} __packed;
> -
>  
>  /* GPIO */
>  

Why are you deleting these structures that userspace uses?  Are you sure
you can do this?  How was this tested?

thanks,

greg k-h