| Message ID | 20220711193743.51456-1-muriloo@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc/kvm: Skip ".." directory in kvmppc_find_cpu_dt | expand |
On 7/11/22 16:37, Murilo Opsfelder Araujo wrote: > Some systems have /proc/device-tree/cpus/../clock-frequency. However, > this is not the expected path for a CPU device tree directory. > > Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> > target/ppc/kvm.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 6eed466f80..c8485a5cc0 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len) > buf[0] = '\0'; > while ((dirp = readdir(dp)) != NULL) { > FILE *f; > + > + /* Don't accidentally read from the upper directory */ > + if (strcmp(dirp->d_name, "..") == 0) { > + continue; > + } > + > snprintf(buf, buf_len, "%s%s/clock-frequency", PROC_DEVTREE_CPU, > dirp->d_name); > f = fopen(buf, "r");
On Mon, Jul 11, 2022 at 04:37:43PM -0300, Murilo Opsfelder Araujo wrote: > Some systems have /proc/device-tree/cpus/../clock-frequency. However, > this is not the expected path for a CPU device tree directory. > > Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > target/ppc/kvm.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 6eed466f80..c8485a5cc0 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len) > buf[0] = '\0'; > while ((dirp = readdir(dp)) != NULL) { > FILE *f; > + > + /* Don't accidentally read from the upper directory */ > + if (strcmp(dirp->d_name, "..") == 0) { It might not be causing problems now, but it would be technically more correct to also skip ".", wouldn't it? > + continue; > + } > + > snprintf(buf, buf_len, "%s%s/clock-frequency", PROC_DEVTREE_CPU, > dirp->d_name); > f = fopen(buf, "r");
On 7/12/22 00:46, David Gibson wrote: > On Mon, Jul 11, 2022 at 04:37:43PM -0300, Murilo Opsfelder Araujo wrote: >> Some systems have /proc/device-tree/cpus/../clock-frequency. However, >> this is not the expected path for a CPU device tree directory. >> >> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> >> --- >> target/ppc/kvm.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c >> index 6eed466f80..c8485a5cc0 100644 >> --- a/target/ppc/kvm.c >> +++ b/target/ppc/kvm.c >> @@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len) >> buf[0] = '\0'; >> while ((dirp = readdir(dp)) != NULL) { >> FILE *f; >> + >> + /* Don't accidentally read from the upper directory */ >> + if (strcmp(dirp->d_name, "..") == 0) { > > It might not be causing problems now, but it would be technically more > correct to also skip ".", wouldn't it? Given that the use of this function is inside kvmppc_read_int_cpu_dt(), which is used to read a property that belongs to a CPU node, I believe you're right. It's better to avoid returning "PROC_DEVTREE_CPU" as well. Murilo, can you please re-send it skipping both ".." and "." ? Better be on the safe side. Daniel > >> + continue; >> + } >> + >> snprintf(buf, buf_len, "%s%s/clock-frequency", PROC_DEVTREE_CPU, >> dirp->d_name); >> f = fopen(buf, "r"); >
Hi, Daniel, David. On 7/12/22 10:03, Daniel Henrique Barboza wrote: > > > On 7/12/22 00:46, David Gibson wrote: >> On Mon, Jul 11, 2022 at 04:37:43PM -0300, Murilo Opsfelder Araujo wrote: >>> Some systems have /proc/device-tree/cpus/../clock-frequency. However, >>> this is not the expected path for a CPU device tree directory. >>> >>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> >>> --- >>> target/ppc/kvm.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c >>> index 6eed466f80..c8485a5cc0 100644 >>> --- a/target/ppc/kvm.c >>> +++ b/target/ppc/kvm.c >>> @@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len) >>> buf[0] = '\0'; >>> while ((dirp = readdir(dp)) != NULL) { >>> FILE *f; >>> + >>> + /* Don't accidentally read from the upper directory */ >>> + if (strcmp(dirp->d_name, "..") == 0) { >> >> It might not be causing problems now, but it would be technically more >> correct to also skip ".", wouldn't it? > > Given that the use of this function is inside kvmppc_read_int_cpu_dt(), which > is used to read a property that belongs to a CPU node, I believe you're right. > It's better to avoid returning "PROC_DEVTREE_CPU" as well. > > Murilo, can you please re-send it skipping both ".." and "." ? Better be > on the safe side. > > > Daniel I've sent v2: https://lore.kernel.org/qemu-devel/20220712210810.35514-1-muriloo@linux.ibm.com/ Thank you for reviewing.
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c index 6eed466f80..c8485a5cc0 100644 --- a/target/ppc/kvm.c +++ b/target/ppc/kvm.c @@ -1877,6 +1877,12 @@ static int kvmppc_find_cpu_dt(char *buf, int buf_len) buf[0] = '\0'; while ((dirp = readdir(dp)) != NULL) { FILE *f; + + /* Don't accidentally read from the upper directory */ + if (strcmp(dirp->d_name, "..") == 0) { + continue; + } + snprintf(buf, buf_len, "%s%s/clock-frequency", PROC_DEVTREE_CPU, dirp->d_name); f = fopen(buf, "r");