* [PATCH 0/3] x86/intel_rdt: Fixes for schemata parsing issues @ 2017-04-19 23:50 Vikas Shivappa 2017-04-19 23:50 ` [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount Vikas Shivappa ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Vikas Shivappa @ 2017-04-19 23:50 UTC (permalink / raw) To: vikas.shivappa, x86, linux-kernel Cc: sai.praneeth.prakhya, hpa, tglx, mingo, ravi.v.shankar, tony.luck, fenghua.yu, vikas.shivappa This series includes some minor fixes to RDT parsing of the schemata. Patches are based on tip x86/cpu/ [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount 2017-04-19 23:50 [PATCH 0/3] x86/intel_rdt: Fixes for schemata parsing issues Vikas Shivappa @ 2017-04-19 23:50 ` Vikas Shivappa 2017-04-20 14:03 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 2017-04-19 23:50 ` [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input Vikas Shivappa 2017-04-19 23:50 ` [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata Vikas Shivappa 2 siblings, 1 reply; 13+ messages in thread From: Vikas Shivappa @ 2017-04-19 23:50 UTC (permalink / raw) To: vikas.shivappa, x86, linux-kernel Cc: sai.praneeth.prakhya, hpa, tglx, mingo, ravi.v.shankar, tony.luck, fenghua.yu, vikas.shivappa Currently max width of 'resource name' and 'resource data' is being initialized based on 'enabled resources' during boot. But the mount can enable different capable resources at a later time which upsets the tabular format of schemata. Fix this to be based on 'all capable' resources. Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> --- arch/x86/kernel/cpu/intel_rdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c index 731f70a..5b36646 100644 --- a/arch/x86/kernel/cpu/intel_rdt.c +++ b/arch/x86/kernel/cpu/intel_rdt.c @@ -492,7 +492,7 @@ static __init void rdt_init_padding(void) struct rdt_resource *r; int cl; - for_each_enabled_rdt_resource(r) { + for_each_capable_rdt_resource(r) { cl = strlen(r->name); if (cl > max_name_width) max_name_width = cl; -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [tip:x86/cpu] x86/intel_rdt: Fix padding when resource is enabled via mount 2017-04-19 23:50 ` [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount Vikas Shivappa @ 2017-04-20 14:03 ` tip-bot for Vikas Shivappa 0 siblings, 0 replies; 13+ messages in thread From: tip-bot for Vikas Shivappa @ 2017-04-20 14:03 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, sai.praneeth.prakhya, tglx, mingo, vikas.shivappa, hpa Commit-ID: adcbdd70309dba5a12a9d8158deb6a62a6d5fc98 Gitweb: http://git.kernel.org/tip/adcbdd70309dba5a12a9d8158deb6a62a6d5fc98 Author: Vikas Shivappa <vikas.shivappa@linux.intel.com> AuthorDate: Wed, 19 Apr 2017 16:50:02 -0700 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 20 Apr 2017 15:57:59 +0200 x86/intel_rdt: Fix padding when resource is enabled via mount Currently max width of 'resource name' and 'resource data' is being initialized based on 'enabled resources' during boot. But the mount can enable different capable resources at a later time which upsets the tabular format of schemata. Fix this to be based on 'all capable' resources. Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Cc: fenghua.yu@intel.com Cc: tony.luck@intel.com Cc: ravi.v.shankar@intel.com Cc: vikas.shivappa@intel.com Link: http://lkml.kernel.org/r/1492645804-17465-2-git-send-email-vikas.shivappa@linux.intel.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/cpu/intel_rdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c index 731f70a..5b36646 100644 --- a/arch/x86/kernel/cpu/intel_rdt.c +++ b/arch/x86/kernel/cpu/intel_rdt.c @@ -492,7 +492,7 @@ static __init void rdt_init_padding(void) struct rdt_resource *r; int cl; - for_each_enabled_rdt_resource(r) { + for_each_capable_rdt_resource(r) { cl = strlen(r->name); if (cl > max_name_width) max_name_width = cl; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input 2017-04-19 23:50 [PATCH 0/3] x86/intel_rdt: Fixes for schemata parsing issues Vikas Shivappa 2017-04-19 23:50 ` [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount Vikas Shivappa @ 2017-04-19 23:50 ` Vikas Shivappa 2017-04-20 13:51 ` Thomas Gleixner 2017-04-20 14:04 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 2017-04-19 23:50 ` [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata Vikas Shivappa 2 siblings, 2 replies; 13+ messages in thread From: Vikas Shivappa @ 2017-04-19 23:50 UTC (permalink / raw) To: vikas.shivappa, x86, linux-kernel Cc: sai.praneeth.prakhya, hpa, tglx, mingo, ravi.v.shankar, tony.luck, fenghua.yu, vikas.shivappa Schemata is displayed in tabular format which introduces some whitespace to show data in a tabular format. If user wants to input the same data that is displayed, the parsing fails. Trim the leading and trailing whitespace to help parse such data. Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> --- arch/x86/kernel/cpu/intel_rdt_schemata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c index 9467a00..3cfa1ca 100644 --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c @@ -143,7 +143,7 @@ static int parse_line(char *line, struct rdt_resource *r) return -EINVAL; list_for_each_entry(d, &r->domains, list) { if (d->id == dom_id) { - if (r->parse_ctrlval(dom, r, d)) + if (r->parse_ctrlval(strim(dom), r, d)) return -EINVAL; goto next; } @@ -220,7 +220,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, goto out; } for_each_enabled_rdt_resource(r) { - if (!strcmp(resname, r->name) && + if (!strcmp(strim(resname), r->name) && closid < r->num_closid) { ret = parse_line(tok, r); if (ret) -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input 2017-04-19 23:50 ` [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input Vikas Shivappa @ 2017-04-20 13:51 ` Thomas Gleixner 2017-04-20 17:51 ` Shivappa Vikas 2017-04-20 14:04 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 1 sibling, 1 reply; 13+ messages in thread From: Thomas Gleixner @ 2017-04-20 13:51 UTC (permalink / raw) To: Vikas Shivappa Cc: vikas.shivappa, x86, linux-kernel, sai.praneeth.prakhya, hpa, mingo, ravi.v.shankar, tony.luck, fenghua.yu On Wed, 19 Apr 2017, Vikas Shivappa wrote: > Schemata is displayed in tabular format which introduces some whitespace > to show data in a tabular format. If user wants to input the same data > that is displayed, the parsing fails. Trim the leading and trailing > whitespace to help parse such data. > > Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> > Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> > Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> > --- > arch/x86/kernel/cpu/intel_rdt_schemata.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c > index 9467a00..3cfa1ca 100644 > --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c > +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c > @@ -143,7 +143,7 @@ static int parse_line(char *line, struct rdt_resource *r) > return -EINVAL; > list_for_each_entry(d, &r->domains, list) { > if (d->id == dom_id) { > - if (r->parse_ctrlval(dom, r, d)) > + if (r->parse_ctrlval(strim(dom), r, d)) > return -EINVAL; > goto next; > } > @@ -220,7 +220,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, > goto out; > } > for_each_enabled_rdt_resource(r) { > - if (!strcmp(resname, r->name) && > + if (!strcmp(strim(resname), r->name) && Both strims() invocations are just silly. They can be done before the loop once instead of doing them over and over again. Thanks, tglx ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input 2017-04-20 13:51 ` Thomas Gleixner @ 2017-04-20 17:51 ` Shivappa Vikas 2017-04-20 17:52 ` Thomas Gleixner 0 siblings, 1 reply; 13+ messages in thread From: Shivappa Vikas @ 2017-04-20 17:51 UTC (permalink / raw) To: Thomas Gleixner Cc: Vikas Shivappa, vikas.shivappa, x86, linux-kernel, sai.praneeth.prakhya, hpa, mingo, ravi.v.shankar, tony.luck, fenghua.yu On Thu, 20 Apr 2017, Thomas Gleixner wrote: > On Wed, 19 Apr 2017, Vikas Shivappa wrote: > >> Schemata is displayed in tabular format which introduces some whitespace >> to show data in a tabular format. If user wants to input the same data >> that is displayed, the parsing fails. Trim the leading and trailing >> whitespace to help parse such data. >> >> Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> >> Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> >> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> >> --- >> arch/x86/kernel/cpu/intel_rdt_schemata.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c >> index 9467a00..3cfa1ca 100644 >> --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c >> +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c >> @@ -143,7 +143,7 @@ static int parse_line(char *line, struct rdt_resource *r) >> return -EINVAL; >> list_for_each_entry(d, &r->domains, list) { >> if (d->id == dom_id) { >> - if (r->parse_ctrlval(dom, r, d)) >> + if (r->parse_ctrlval(strim(dom), r, d)) >> return -EINVAL; >> goto next; >> } >> @@ -220,7 +220,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, >> goto out; >> } >> for_each_enabled_rdt_resource(r) { >> - if (!strcmp(resname, r->name) && >> + if (!strcmp(strim(resname), r->name) && > > Both strims() invocations are just silly. They can be done before the loop > once instead of doing them over and over again. Will fix. I went overboard not wanting to add a line Thanks, Vikas > > Thanks, > > tglx > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input 2017-04-20 17:51 ` Shivappa Vikas @ 2017-04-20 17:52 ` Thomas Gleixner 2017-04-20 21:19 ` Shivappa Vikas 0 siblings, 1 reply; 13+ messages in thread From: Thomas Gleixner @ 2017-04-20 17:52 UTC (permalink / raw) To: Shivappa Vikas Cc: Vikas Shivappa, x86, linux-kernel, sai.praneeth.prakhya, hpa, mingo, ravi.v.shankar, tony.luck, fenghua.yu On Thu, 20 Apr 2017, Shivappa Vikas wrote: > Will fix. I went overboard not wanting to add a line Finish reading the thread before you start :) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input 2017-04-20 17:52 ` Thomas Gleixner @ 2017-04-20 21:19 ` Shivappa Vikas 0 siblings, 0 replies; 13+ messages in thread From: Shivappa Vikas @ 2017-04-20 21:19 UTC (permalink / raw) To: Thomas Gleixner Cc: Shivappa Vikas, Vikas Shivappa, x86, linux-kernel, sai.praneeth.prakhya, hpa, mingo, ravi.v.shankar, tony.luck, fenghua.yu On Thu, 20 Apr 2017, Thomas Gleixner wrote: > On Thu, 20 Apr 2017, Shivappa Vikas wrote: >> Will fix. I went overboard not wanting to add a line > > Finish reading the thread before you start :) Got it :) Had not seen the tip bot emails on my other email.. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip:x86/cpu] x86/intel_rdt: Trim whitespace while parsing schemata input 2017-04-19 23:50 ` [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input Vikas Shivappa 2017-04-20 13:51 ` Thomas Gleixner @ 2017-04-20 14:04 ` tip-bot for Vikas Shivappa 1 sibling, 0 replies; 13+ messages in thread From: tip-bot for Vikas Shivappa @ 2017-04-20 14:04 UTC (permalink / raw) To: linux-tip-commits Cc: vikas.shivappa, linux-kernel, sai.praneeth.prakhya, mingo, tglx, hpa Commit-ID: 634b0e0491d6f6e882b922eb41c278d01a743bab Gitweb: http://git.kernel.org/tip/634b0e0491d6f6e882b922eb41c278d01a743bab Author: Vikas Shivappa <vikas.shivappa@linux.intel.com> AuthorDate: Wed, 19 Apr 2017 16:50:03 -0700 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 20 Apr 2017 15:57:59 +0200 x86/intel_rdt: Trim whitespace while parsing schemata input Schemata is displayed in tabular format which introduces some whitespace to show data in a tabular format. Writing back the same data fails as the parser does not handle the whitespace. Trim the leading and trailing whitespace before parsing. Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Cc: fenghua.yu@intel.com Cc: tony.luck@intel.com Cc: ravi.v.shankar@intel.com Cc: vikas.shivappa@intel.com Link: http://lkml.kernel.org/r/1492645804-17465-3-git-send-email-vikas.shivappa@linux.intel.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/cpu/intel_rdt_schemata.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c index 9467a00..e64b2cf 100644 --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c @@ -141,6 +141,7 @@ next: id = strsep(&dom, "="); if (!dom || kstrtoul(id, 10, &dom_id)) return -EINVAL; + dom = strim(dom); list_for_each_entry(d, &r->domains, list) { if (d->id == dom_id) { if (r->parse_ctrlval(dom, r, d)) @@ -214,7 +215,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, dom->have_new_ctrl = false; while ((tok = strsep(&buf, "\n")) != NULL) { - resname = strsep(&tok, ":"); + resname = strim(strsep(&tok, ":")); if (!tok) { ret = -EINVAL; goto out; ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata 2017-04-19 23:50 [PATCH 0/3] x86/intel_rdt: Fixes for schemata parsing issues Vikas Shivappa 2017-04-19 23:50 ` [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount Vikas Shivappa 2017-04-19 23:50 ` [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input Vikas Shivappa @ 2017-04-19 23:50 ` Vikas Shivappa 2017-04-20 13:44 ` Thomas Gleixner 2017-04-20 14:04 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 2 siblings, 2 replies; 13+ messages in thread From: Vikas Shivappa @ 2017-04-19 23:50 UTC (permalink / raw) To: vikas.shivappa, x86, linux-kernel Cc: sai.praneeth.prakhya, hpa, tglx, mingo, ravi.v.shankar, tony.luck, fenghua.yu, vikas.shivappa When schemata parses the resource names, currently it may not return error for incorrect resource names and fail quietly without updating the control values. This is because for_each_enabled_rdt_resource(r) leaves "r" pointing beyond the end of the rdt_resources_all[] array, and we check for !r->name which results in an out of bounds access and also may not be NULL, hence we may not detect that we did not find the name user supplied. Update this to check (r == (rdt_resources_all + RDT_NUM_RESOURCES)) instead. Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> --- arch/x86/kernel/cpu/intel_rdt_schemata.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c index 3cfa1ca..bb99bd8 100644 --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c @@ -228,7 +228,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, break; } } - if (!r->name) { + if (r == (rdt_resources_all + RDT_NUM_RESOURCES)) { ret = -EINVAL; goto out; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata 2017-04-19 23:50 ` [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata Vikas Shivappa @ 2017-04-20 13:44 ` Thomas Gleixner 2017-04-20 18:57 ` Shivappa Vikas 2017-04-20 14:04 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 1 sibling, 1 reply; 13+ messages in thread From: Thomas Gleixner @ 2017-04-20 13:44 UTC (permalink / raw) To: Vikas Shivappa Cc: vikas.shivappa, x86, linux-kernel, sai.praneeth.prakhya, hpa, mingo, ravi.v.shankar, tony.luck, fenghua.yu On Wed, 19 Apr 2017, Vikas Shivappa wrote: > When schemata parses the resource names, currently it may not return > error for incorrect resource names and fail quietly without updating the > control values. > > This is because for_each_enabled_rdt_resource(r) leaves "r" pointing > beyond the end of the rdt_resources_all[] array, and we check for > !r->name which results in an out of bounds access and also may not be > NULL, hence we may not detect that we did not find the name user > supplied. > > Update this to check (r == (rdt_resources_all + RDT_NUM_RESOURCES)) > instead. > > Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> > Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> > Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> > --- > arch/x86/kernel/cpu/intel_rdt_schemata.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c > index 3cfa1ca..bb99bd8 100644 > --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c > +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c > @@ -228,7 +228,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, > break; > } > } > - if (!r->name) { > + if (r == (rdt_resources_all + RDT_NUM_RESOURCES)) { > ret = -EINVAL; > goto out; > } The resulting loop is just horrible to read. We can do better than that. Patch below. Thanks, tglx 8<------------- --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c @@ -187,6 +187,17 @@ static int update_domains(struct rdt_res return 0; } +static int rdtgroup_parse_resource(char *resname, char *tok, int closid) +{ + struct rdt_resource *r; + + for_each_enabled_rdt_resource(r) { + if (!strcmp(resname, r->name) && closid < r->num_closid) + return parse_line(tok, r); + } + return -EINVAL; +} + ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { @@ -209,9 +220,10 @@ ssize_t rdtgroup_schemata_write(struct k closid = rdtgrp->closid; - for_each_enabled_rdt_resource(r) + for_each_enabled_rdt_resource(r) { list_for_each_entry(dom, &r->domains, list) dom->have_new_ctrl = false; + } while ((tok = strsep(&buf, "\n")) != NULL) { resname = strsep(&tok, ":"); @@ -219,19 +231,9 @@ ssize_t rdtgroup_schemata_write(struct k ret = -EINVAL; goto out; } - for_each_enabled_rdt_resource(r) { - if (!strcmp(strim(resname), r->name) && - closid < r->num_closid) { - ret = parse_line(tok, r); - if (ret) - goto out; - break; - } - } - if (!r->name) { - ret = -EINVAL; + ret = rdtgroup_parse_resource(strim(resname), tok, closid); + if (ret) goto out; - } } for_each_enabled_rdt_resource(r) { ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata 2017-04-20 13:44 ` Thomas Gleixner @ 2017-04-20 18:57 ` Shivappa Vikas 0 siblings, 0 replies; 13+ messages in thread From: Shivappa Vikas @ 2017-04-20 18:57 UTC (permalink / raw) To: Thomas Gleixner Cc: Vikas Shivappa, vikas.shivappa, x86, linux-kernel, sai.praneeth.prakhya, hpa, mingo, Shankar, Ravi V, tony.luck, Yu, Fenghua On Thu, 20 Apr 2017, Thomas Gleixner wrote: > On Wed, 19 Apr 2017, Vikas Shivappa wrote: > >> } > > The resulting loop is just horrible to read. We can do better than > that. Patch below. > > Thanks, > > tglx > 8<------------- > > --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c > +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c > @@ -187,6 +187,17 @@ static int update_domains(struct rdt_res > return 0; > } > > +static int rdtgroup_parse_resource(char *resname, char *tok, int closid) > +{ > + struct rdt_resource *r; > + > + for_each_enabled_rdt_resource(r) { > + if (!strcmp(resname, r->name) && closid < r->num_closid) > + return parse_line(tok, r); > + } > + return -EINVAL; > +} > + > ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, > char *buf, size_t nbytes, loff_t off) > { > @@ -209,9 +220,10 @@ ssize_t rdtgroup_schemata_write(struct k > > closid = rdtgrp->closid; > > - for_each_enabled_rdt_resource(r) > + for_each_enabled_rdt_resource(r) { > list_for_each_entry(dom, &r->domains, list) > dom->have_new_ctrl = false; > + } > > while ((tok = strsep(&buf, "\n")) != NULL) { > resname = strsep(&tok, ":"); > @@ -219,19 +231,9 @@ ssize_t rdtgroup_schemata_write(struct k > ret = -EINVAL; > goto out; > } > - for_each_enabled_rdt_resource(r) { > - if (!strcmp(strim(resname), r->name) && > - closid < r->num_closid) { > - ret = parse_line(tok, r); > - if (ret) > - goto out; > - break; > - } > - } > - if (!r->name) { > - ret = -EINVAL; > + ret = rdtgroup_parse_resource(strim(resname), tok, closid); > + if (ret) > goto out; > - } > } Ok agree its better way. I was trying to add a NULL which Tony already said was Nacked, did not really want to use a pointer which may be out of bounds although we dont check the contents. Thanks, Vikas ^ permalink raw reply [flat|nested] 13+ messages in thread
* [tip:x86/cpu] x86/intel_rdt: Return error for incorrect resource names in schemata 2017-04-19 23:50 ` [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata Vikas Shivappa 2017-04-20 13:44 ` Thomas Gleixner @ 2017-04-20 14:04 ` tip-bot for Vikas Shivappa 1 sibling, 0 replies; 13+ messages in thread From: tip-bot for Vikas Shivappa @ 2017-04-20 14:04 UTC (permalink / raw) To: linux-tip-commits Cc: vikas.shivappa, sai.praneeth.prakhya, hpa, linux-kernel, tglx, mingo Commit-ID: 4797b7dfdfcf457075c36743d71e2b0feeaaa20f Gitweb: http://git.kernel.org/tip/4797b7dfdfcf457075c36743d71e2b0feeaaa20f Author: Vikas Shivappa <vikas.shivappa@linux.intel.com> AuthorDate: Wed, 19 Apr 2017 16:50:04 -0700 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Thu, 20 Apr 2017 15:57:59 +0200 x86/intel_rdt: Return error for incorrect resource names in schemata When schemata parses the resource names it does not return an error if it detects incorrect resource names and fails quietly. This happens because for_each_enabled_rdt_resource(r) leaves "r" pointing beyond the end of the rdt_resources_all[] array, and the check for !r->name results in an out of bounds access. Split the resource parsing part into a helper function to avoid the issue. [ tglx: Made it readable by splitting the parser loop out into a function ] Reported-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> Tested-by: Prakhya, Sai Praneeth <sai.praneeth.prakhya@intel.com> Cc: fenghua.yu@intel.com Cc: tony.luck@intel.com Cc: ravi.v.shankar@intel.com Cc: vikas.shivappa@intel.com Link: http://lkml.kernel.org/r/1492645804-17465-4-git-send-email-vikas.shivappa@linux.intel.com Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/cpu/intel_rdt_schemata.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/arch/x86/kernel/cpu/intel_rdt_schemata.c b/arch/x86/kernel/cpu/intel_rdt_schemata.c index e64b2cf..406d7a6 100644 --- a/arch/x86/kernel/cpu/intel_rdt_schemata.c +++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c @@ -188,6 +188,17 @@ done: return 0; } +static int rdtgroup_parse_resource(char *resname, char *tok, int closid) +{ + struct rdt_resource *r; + + for_each_enabled_rdt_resource(r) { + if (!strcmp(resname, r->name) && closid < r->num_closid) + return parse_line(tok, r); + } + return -EINVAL; +} + ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { @@ -210,9 +221,10 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, closid = rdtgrp->closid; - for_each_enabled_rdt_resource(r) + for_each_enabled_rdt_resource(r) { list_for_each_entry(dom, &r->domains, list) dom->have_new_ctrl = false; + } while ((tok = strsep(&buf, "\n")) != NULL) { resname = strim(strsep(&tok, ":")); @@ -220,19 +232,9 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of, ret = -EINVAL; goto out; } - for_each_enabled_rdt_resource(r) { - if (!strcmp(resname, r->name) && - closid < r->num_closid) { - ret = parse_line(tok, r); - if (ret) - goto out; - break; - } - } - if (!r->name) { - ret = -EINVAL; + ret = rdtgroup_parse_resource(resname, tok, closid); + if (ret) goto out; - } } for_each_enabled_rdt_resource(r) { ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-04-20 21:19 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-19 23:50 [PATCH 0/3] x86/intel_rdt: Fixes for schemata parsing issues Vikas Shivappa 2017-04-19 23:50 ` [PATCH 1/3] x86/intel_rdt: Fix padding when resource is enabled via mount Vikas Shivappa 2017-04-20 14:03 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 2017-04-19 23:50 ` [PATCH 2/3] x86/intel_rdt: Trim whitespace while parsing schemata input Vikas Shivappa 2017-04-20 13:51 ` Thomas Gleixner 2017-04-20 17:51 ` Shivappa Vikas 2017-04-20 17:52 ` Thomas Gleixner 2017-04-20 21:19 ` Shivappa Vikas 2017-04-20 14:04 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa 2017-04-19 23:50 ` [PATCH 3/3] x86/intel_rdt: Return error for incorrect resource names in schemata Vikas Shivappa 2017-04-20 13:44 ` Thomas Gleixner 2017-04-20 18:57 ` Shivappa Vikas 2017-04-20 14:04 ` [tip:x86/cpu] " tip-bot for Vikas Shivappa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).