* [patch] [media] media: info leak in media_device_enum_entities()
@ 2013-04-21 11:10 Dan Carpenter
2013-04-21 11:51 ` walter harms
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2013-04-21 11:10 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, kernel-janitors
The last part of the "u_ent.name" buffer isn't cleared so it still has
uninitialized stack memory.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 99b80b6..1957c0d 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -102,9 +102,12 @@ static long media_device_enum_entities(struct media_device *mdev,
return -EINVAL;
u_ent.id = ent->id;
- u_ent.name[0] = '\0';
- if (ent->name)
- strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
+ if (ent->name) {
+ strncpy(u_ent.name, ent->name, sizeof(u_ent.name));
+ u_ent.name[sizeof(u_ent.name) - 1] = '\0';
+ } else {
+ memset(u_ent.name, 0, sizeof(u_ent.name));
+ }
u_ent.type = ent->type;
u_ent.revision = ent->revision;
u_ent.flags = ent->flags;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [patch] [media] media: info leak in media_device_enum_entities()
2013-04-21 11:10 [patch] [media] media: info leak in media_device_enum_entities() Dan Carpenter
@ 2013-04-21 11:51 ` walter harms
2013-04-21 16:42 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: walter harms @ 2013-04-21 11:51 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Mauro Carvalho Chehab, linux-media, kernel-janitors
Am 21.04.2013 13:10, schrieb Dan Carpenter:
> The last part of the "u_ent.name" buffer isn't cleared so it still has
> uninitialized stack memory.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 99b80b6..1957c0d 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -102,9 +102,12 @@ static long media_device_enum_entities(struct media_device *mdev,
> return -EINVAL;
>
> u_ent.id = ent->id;
> - u_ent.name[0] = '\0';
> - if (ent->name)
> - strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
> + if (ent->name) {
> + strncpy(u_ent.name, ent->name, sizeof(u_ent.name));
> + u_ent.name[sizeof(u_ent.name) - 1] = '\0';
> + } else {
> + memset(u_ent.name, 0, sizeof(u_ent.name));
> + }
I would always memset()
and then do strncpy() for sizeof(u_ent.name) - 1
the rest is always zero.
re,
wh
> u_ent.type = ent->type;
> u_ent.revision = ent->revision;
> u_ent.flags = ent->flags;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch] [media] media: info leak in media_device_enum_entities()
2013-04-21 11:51 ` walter harms
@ 2013-04-21 16:42 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-04-21 16:42 UTC (permalink / raw)
To: walter harms; +Cc: Mauro Carvalho Chehab, linux-media, kernel-janitors
On Sun, Apr 21, 2013 at 01:51:56PM +0200, walter harms wrote:
>
>
> Am 21.04.2013 13:10, schrieb Dan Carpenter:
> > The last part of the "u_ent.name" buffer isn't cleared so it still has
> > uninitialized stack memory.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> > index 99b80b6..1957c0d 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -102,9 +102,12 @@ static long media_device_enum_entities(struct media_device *mdev,
> > return -EINVAL;
> >
> > u_ent.id = ent->id;
> > - u_ent.name[0] = '\0';
> > - if (ent->name)
> > - strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
> > + if (ent->name) {
> > + strncpy(u_ent.name, ent->name, sizeof(u_ent.name));
> > + u_ent.name[sizeof(u_ent.name) - 1] = '\0';
> > + } else {
> > + memset(u_ent.name, 0, sizeof(u_ent.name));
> > + }
>
> I would always memset()
> and then do strncpy() for sizeof(u_ent.name) - 1
> the rest is always zero.
Both ways are fine. You'd still have to test for "if (ent->name)",
of course. This way is a little faster because I do the test first.
Mauro, if you want I can redo it?
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-04-21 16:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-21 11:10 [patch] [media] media: info leak in media_device_enum_entities() Dan Carpenter
2013-04-21 11:51 ` walter harms
2013-04-21 16:42 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox