| Message ID | 20210120140454.4286-2-nsaenzjulienne@suse.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Console/stdio use after free | expand |
Hi Nicolas, On Wed, 20 Jan 2021 at 07:05, Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > stdio_valid() will confirm that a struct stdio_dev pointer is indeed > valid. > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > --- > common/stdio.c | 11 +++++++++++ > include/stdio_dev.h | 1 + > 2 files changed, 12 insertions(+) > > diff --git a/common/stdio.c b/common/stdio.c > index abf9b1e915..69b7d2692d 100644 > --- a/common/stdio.c > +++ b/common/stdio.c > @@ -157,6 +157,17 @@ static int stdio_probe_device(const char *name, enum uclass_id id, > return 0; > } > > +bool stdio_valid(struct stdio_dev *dev) > +{ > + struct stdio_dev *sdev; > + > + list_for_each_entry(sdev, &devs.list, list) > + if (sdev == dev) > + return true; > + > + return false; > +} > + > struct stdio_dev *stdio_get_by_name(const char *name) > { > struct list_head *pos; > diff --git a/include/stdio_dev.h b/include/stdio_dev.h > index 48871a6a22..f341439b03 100644 > --- a/include/stdio_dev.h > +++ b/include/stdio_dev.h > @@ -97,6 +97,7 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force); > struct list_head *stdio_get_list(void); > struct stdio_dev *stdio_get_by_name(const char *name); > struct stdio_dev *stdio_clone(struct stdio_dev *dev); > +bool stdio_valid(struct stdio_dev *dev); Please add a full function comment and explain what valid means. > > int drv_lcd_init(void); > int drv_video_init(void); > -- > 2.30.0 > Regards, Simon
On Sat, 2021-01-23 at 19:03 -0700, Simon Glass wrote: > Hi Nicolas, > > On Wed, 20 Jan 2021 at 07:05, Nicolas Saenz Julienne > <nsaenzjulienne@suse.de> wrote: > > > > stdio_valid() will confirm that a struct stdio_dev pointer is indeed > > valid. > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > --- > > common/stdio.c | 11 +++++++++++ > > include/stdio_dev.h | 1 + > > 2 files changed, 12 insertions(+) > > > > diff --git a/common/stdio.c b/common/stdio.c > > index abf9b1e915..69b7d2692d 100644 > > --- a/common/stdio.c > > +++ b/common/stdio.c > > @@ -157,6 +157,17 @@ static int stdio_probe_device(const char *name, enum uclass_id id, > > return 0; > > } > > > > +bool stdio_valid(struct stdio_dev *dev) > > +{ > > + struct stdio_dev *sdev; > > + > > + list_for_each_entry(sdev, &devs.list, list) > > + if (sdev == dev) > > + return true; > > + > > + return false; > > +} > > + > > struct stdio_dev *stdio_get_by_name(const char *name) > > { > > struct list_head *pos; > > diff --git a/include/stdio_dev.h b/include/stdio_dev.h > > index 48871a6a22..f341439b03 100644 > > --- a/include/stdio_dev.h > > +++ b/include/stdio_dev.h > > @@ -97,6 +97,7 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force); > > struct list_head *stdio_get_list(void); > > struct stdio_dev *stdio_get_by_name(const char *name); > > struct stdio_dev *stdio_clone(struct stdio_dev *dev); > > +bool stdio_valid(struct stdio_dev *dev); > > Please add a full function comment and explain what valid means. As discussed with Andy, this is a workaround that doesn't address the underlying issue. If it's good enough for the time being I'll be happy to send a v2. I'll leave a comment stating that it's something to fix. Regards, Nicolas
On Mon, Jan 25, 2021 at 05:34:05PM +0100, Nicolas Saenz Julienne wrote: > On Sat, 2021-01-23 at 19:03 -0700, Simon Glass wrote: > > Hi Nicolas, > > > > On Wed, 20 Jan 2021 at 07:05, Nicolas Saenz Julienne > > <nsaenzjulienne@suse.de> wrote: > > > > > > stdio_valid() will confirm that a struct stdio_dev pointer is indeed > > > valid. > > > > > > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > > --- > > > common/stdio.c | 11 +++++++++++ > > > include/stdio_dev.h | 1 + > > > 2 files changed, 12 insertions(+) > > > > > > diff --git a/common/stdio.c b/common/stdio.c > > > index abf9b1e915..69b7d2692d 100644 > > > --- a/common/stdio.c > > > +++ b/common/stdio.c > > > @@ -157,6 +157,17 @@ static int stdio_probe_device(const char *name, enum uclass_id id, > > > return 0; > > > } > > > > > > +bool stdio_valid(struct stdio_dev *dev) > > > +{ > > > + struct stdio_dev *sdev; > > > + > > > + list_for_each_entry(sdev, &devs.list, list) > > > + if (sdev == dev) > > > + return true; > > > + > > > + return false; > > > +} > > > + > > > struct stdio_dev *stdio_get_by_name(const char *name) > > > { > > > struct list_head *pos; > > > diff --git a/include/stdio_dev.h b/include/stdio_dev.h > > > index 48871a6a22..f341439b03 100644 > > > --- a/include/stdio_dev.h > > > +++ b/include/stdio_dev.h > > > @@ -97,6 +97,7 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force); > > > struct list_head *stdio_get_list(void); > > > struct stdio_dev *stdio_get_by_name(const char *name); > > > struct stdio_dev *stdio_clone(struct stdio_dev *dev); > > > +bool stdio_valid(struct stdio_dev *dev); > > > > Please add a full function comment and explain what valid means. > > As discussed with Andy, this is a workaround that doesn't address the > underlying issue. If it's good enough for the time being I'll be happy to send > a v2. > > I'll leave a comment stating that it's something to fix. Please do, thanks.
diff --git a/common/stdio.c b/common/stdio.c index abf9b1e915..69b7d2692d 100644 --- a/common/stdio.c +++ b/common/stdio.c @@ -157,6 +157,17 @@ static int stdio_probe_device(const char *name, enum uclass_id id, return 0; } +bool stdio_valid(struct stdio_dev *dev) +{ + struct stdio_dev *sdev; + + list_for_each_entry(sdev, &devs.list, list) + if (sdev == dev) + return true; + + return false; +} + struct stdio_dev *stdio_get_by_name(const char *name) { struct list_head *pos; diff --git a/include/stdio_dev.h b/include/stdio_dev.h index 48871a6a22..f341439b03 100644 --- a/include/stdio_dev.h +++ b/include/stdio_dev.h @@ -97,6 +97,7 @@ int stdio_deregister_dev(struct stdio_dev *dev, int force); struct list_head *stdio_get_list(void); struct stdio_dev *stdio_get_by_name(const char *name); struct stdio_dev *stdio_clone(struct stdio_dev *dev); +bool stdio_valid(struct stdio_dev *dev); int drv_lcd_init(void); int drv_video_init(void);
stdio_valid() will confirm that a struct stdio_dev pointer is indeed valid. Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> --- common/stdio.c | 11 +++++++++++ include/stdio_dev.h | 1 + 2 files changed, 12 insertions(+)