staging: strncpy issue, need always let NUL terminated string ended by zero.

PATCH In review Chen Gang 2 participants 4 messages May 7, 2013 – Jun 8, 2013

Subsystems (2)

Media (V4L/DVB) Staging

lore

Share & tools

lore

Chen Gang

author

2013-05-07 12:11 UTC #0 lore raw

PATCH 1/1 staging: strncpy issue, need always let NUL terminated string ended by zero.

For NUL terminated string, need always let it ended by zero.

The 'name' may be copied to user mode ('dvb_fe->ops.info' is 'struct dvb_frontend_info' which is defined in ./include/uapi/...), and its length is also known within as102_dvb_register_fe(), so need fully initialize it (not use strlcpy instead of strncpy).

Signed-off-by Chen Gang <[email protected]>

---
 drivers/staging/media/as102/as102_fe.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/staging/media/as102/as102_fe.c +1 -0

index 9ce8c9d..b3efec9 100644
@@ -334,6 +334,7 @@ int as102_dvb_register_fe(struct as102_dev_t *as102_dev,
 	memcpy(&dvb_fe->ops, &as102_fe_ops, sizeof(struct dvb_frontend_ops));
 	strncpy(dvb_fe->ops.info.name, as102_dev->name,
 		sizeof(dvb_fe->ops.info.name));
+	dvb_fe->ops.info.name[sizeof(dvb_fe->ops.info.name) - 1] = '\0';
 
 	/* register dvb frontend */
 	errno = dvb_register_frontend(dvb_adap, dvb_fe);

Signature · 1 line

1.7.7.6

Mauro Carvalho Chehab

2013-05-21 11:47 UTC #1 lore raw

Em Tue, 07 May 2013 20:11:08 +0800
Chen Gang <[email protected]> escreveu:

Quoted 19 lines

For NUL terminated string, need always let it ended by zero.
The 'name' may be copied to user mode ('dvb_fe->ops.info' is 'struct dvb_frontend_info' which is defined in ./include/uapi/...), and its length is also known within as102_dvb_register_fe(), so need fully initialize it (not use strlcpy instead of strncpy).
Signed-off-by Chen Gang <[email protected]>
---
 drivers/staging/media/as102/as102_fe.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/staging/media/as102/as102_fe.c +0 -0
index 9ce8c9d..b3efec9 100644

drivers/staging/media/as102/as102_fe.c int as102_dvb_register_fe(struct as102_dev_t *as102_dev, · line 337

Quoted 4 lines

drivers/staging/media/as102/as102_fe.c +1 -0

@@ -334,6 +334,7 @@ int as102_dvb_register_fe(struct as102_dev_t *as102_dev,
 	memcpy(&dvb_fe->ops, &as102_fe_ops, sizeof(struct dvb_frontend_ops));
 	strncpy(dvb_fe->ops.info.name, as102_dev->name,
 		sizeof(dvb_fe->ops.info.name));
+	dvb_fe->ops.info.name[sizeof(dvb_fe->ops.info.name) - 1] = '\0';

Instead, the better would be to use strlcpy(), as it warrants that the copied string will be nul-terminated.

Regards,
Mauro

Chen Gang

author

2013-05-21 11:54 UTC #2 lore raw

Quoted 17 lines · Mauro Carvalho Chehab

Em Tue, 07 May 2013 20:11:08 +0800
Chen Gang <[email protected]> escreveu:
Quoted 14 lines
Quoted 14 lines
For NUL terminated string, need always let it ended by zero.
The 'name' may be copied to user mode ('dvb_fe->ops.info' is 'struct dvb_frontend_info' which is defined in ./include/uapi/...), and its length is also known within as102_dvb_register_fe(), so need fully initialize it (not use strlcpy instead of strncpy).
Signed-off-by Chen Gang <[email protected]>
---
 drivers/staging/media/as102/as102_fe.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Quoted 11 lines

Quoted 9 lines
drivers/staging/media/as102/as102_fe.c int as102_dvb_register_fe(struct as102_dev_t *as102_dev, · line 337
Quoted 9 lines
drivers/staging/media/as102/as102_fe.c +1 -0
index 9ce8c9d..b3efec9 100644
@@ -334,6 +334,7 @@ int as102_dvb_register_fe(struct as102_dev_t *as102_dev,
 	memcpy(&dvb_fe->ops, &as102_fe_ops, sizeof(struct dvb_frontend_ops));
 	strncpy(dvb_fe->ops.info.name, as102_dev->name,
 		sizeof(dvb_fe->ops.info.name));
+	dvb_fe->ops.info.name[sizeof(dvb_fe->ops.info.name) - 1] = '\0';
Instead, the better would be to use strlcpy(), as it warrants that the copied string will be nul-terminated.

Within this function, we know 'dvb_fe->ops.info' my copy to user mode (the structure is defined in ./include/uapi/...), and we also known the full length of the buffer, so better still use strncpy to give a full initialized, and still be sure of the nul-terminated.

Thanks.

Signature · 3 lines

Chen Gang

Asianux Corporation

Chen Gang

author

2013-06-08 04:36 UTC #3 lore raw

Hello Maintainers:

Please help check it when you have time.

Thanks.

Quoted 38 lines · Chen Gang

Quoted 17 lines · Mauro Carvalho Chehab
Em Tue, 07 May 2013 20:11:08 +0800
Chen Gang <[email protected]> escreveu:
Quoted 14 lines
Quoted 14 lines
For NUL terminated string, need always let it ended by zero.
The 'name' may be copied to user mode ('dvb_fe->ops.info' is 'struct dvb_frontend_info' which is defined in ./include/uapi/...), and its length is also known within as102_dvb_register_fe(), so need fully initialize it (not use strlcpy instead of strncpy).
Signed-off-by Chen Gang <[email protected]>
---
 drivers/staging/media/as102/as102_fe.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Quoted 11 lines
Quoted 9 lines
drivers/staging/media/as102/as102_fe.c int as102_dvb_register_fe(struct as102_dev_t *as102_dev, · line 337
Quoted 9 lines
drivers/staging/media/as102/as102_fe.c +1 -0
index 9ce8c9d..b3efec9 100644
@@ -334,6 +334,7 @@ int as102_dvb_register_fe(struct as102_dev_t *as102_dev,
 	memcpy(&dvb_fe->ops, &as102_fe_ops, sizeof(struct dvb_frontend_ops));
 	strncpy(dvb_fe->ops.info.name, as102_dev->name,
 		sizeof(dvb_fe->ops.info.name));
+	dvb_fe->ops.info.name[sizeof(dvb_fe->ops.info.name) - 1] = '\0';
Instead, the better would be to use strlcpy(), as it warrants that the copied string will be nul-terminated.
Within this function, we know 'dvb_fe->ops.info' my copy to user mode (the structure is defined in ./include/uapi/...), and we also known the full length of the buffer, so better still use strncpy to give a full initialized, and still be sure of the nul-terminated.
Thanks.

Signature · 3 lines

Chen Gang

Asianux Corporation