| Message ID | 20200221165243.25100-1-jeyu@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4) | expand |
+++ Jessica Yu [21/02/20 17:52 +0100]: >depmod -e -E is broken for kernel versions >= 5.4, because a new >namespace field was added to Module.symvers. Each line is tab delimited >with 5 fields in total. E.g., > > 0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL > >When a symbol doesn't have a namespace, then the namespace field is empty: > > 0xb352177e\tfind_first_bit\t\tvmlinux\tEXPORT_SYMBOL > >Fix up depmod_load_symvers() so it finds the crc, symbol, and module >("where") fields correctly. Since there can be empty fields, strsep() is >used instead of strtok(), since strtok() cannot handle empty fields >(i.e., two delimiters in succession). > >Signed-off-by: Jessica Yu <jeyu@kernel.org> Friendly ping? :-) Thanks, Jessica
On Fri, Feb 21, 2020 at 8:53 AM Jessica Yu <jeyu@kernel.org> wrote: > > depmod -e -E is broken for kernel versions >= 5.4, because a new > namespace field was added to Module.symvers. Each line is tab delimited > with 5 fields in total. E.g., > > 0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL > > When a symbol doesn't have a namespace, then the namespace field is empty: > > 0xb352177e\tfind_first_bit\t\tvmlinux\tEXPORT_SYMBOL Why is namespace added in the *middle*? We remember we specifically talked about compatibility back when this was added. Why is it not at the end so tools that don't know about namespace don't stop working? Lucas De Marchi > > Fix up depmod_load_symvers() so it finds the crc, symbol, and module > ("where") fields correctly. Since there can be empty fields, strsep() is > used instead of strtok(), since strtok() cannot handle empty fields > (i.e., two delimiters in succession). > > Signed-off-by: Jessica Yu <jeyu@kernel.org> > --- > > Hi, > > I was not sure what the best way of determining the symvers format was. I > guess counting delimiters isn't the prettiest way, but if anyone has a > better idea, let me know. Thanks! > > tools/depmod.c | 35 ++++++++++++++++++++++++++++++----- > 1 file changed, 30 insertions(+), 5 deletions(-) > > diff --git a/tools/depmod.c b/tools/depmod.c > index fbbce10..c5b9612 100644 > --- a/tools/depmod.c > +++ b/tools/depmod.c > @@ -2577,7 +2577,9 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename) > { > char line[10240]; > FILE *fp; > - unsigned int linenum = 0; > + unsigned int linenum = 0, cnt = 0; > + bool uses_namespaces = false; > + char *ptr; > > fp = fopen(filename, "r"); > if (fp == NULL) { > @@ -2587,7 +2589,26 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename) > } > DBG("load symvers: %s\n", filename); > > - /* eg. "0xb352177e\tfind_first_bit\tvmlinux\tEXPORT_SYMBOL" */ > + /* > + * First, check for >=5.4 Module.symvers format: > + * "0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL" > + * If there are 5 fields (4 tabs), assume we're using the new format. > + */ > + fgets(line, sizeof(line), fp); > + ptr = line; > + while ((ptr = strchr(ptr, '\t')) != NULL) { > + cnt++; > + ptr++; > + } > + if (cnt > 3) > + uses_namespaces = true; > + rewind(fp); > + > + /* > + * eg. "0xb352177e\tfind_first_bit\tvmlinux\tEXPORT_SYMBOL" > + * Or if kernel version >=5.4: > + * "0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL" > + */ > while (fgets(line, sizeof(line), fp) != NULL) { > const char *ver, *sym, *where; > char *verend; > @@ -2595,9 +2616,13 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename) > > linenum++; > > - ver = strtok(line, " \t"); > - sym = strtok(NULL, " \t"); > - where = strtok(NULL, " \t"); > + ptr = (char *)line; > + ver = strsep(&ptr, "\t"); > + sym = strsep(&ptr, "\t"); > + if (uses_namespaces) /* skip namespace field */ > + strsep(&ptr, "\t"); > + where = strsep(&ptr, "\t"); > + > if (!ver || !sym || !where) > continue; > > -- > 2.16.4 >
+++ Lucas De Marchi [03/03/20 22:28 -0800]: >On Fri, Feb 21, 2020 at 8:53 AM Jessica Yu <jeyu@kernel.org> wrote: >> >> depmod -e -E is broken for kernel versions >= 5.4, because a new >> namespace field was added to Module.symvers. Each line is tab delimited >> with 5 fields in total. E.g., >> >> 0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL >> >> When a symbol doesn't have a namespace, then the namespace field is empty: >> >> 0xb352177e\tfind_first_bit\t\tvmlinux\tEXPORT_SYMBOL > >Why is namespace added in the *middle*? We remember we specifically >talked about compatibility back when this was added. Why is it not at >the end so tools that don't know about namespace don't stop working? > >Lucas De Marchi Sigh, I do remember we had a short discussion upstream back in August [1] when we were tossing around Module.symvers format preferences. It is my fault for not having pushed the backwards compatibility option more instead of thinking it could be patched up in kmod tools. I think maybe the best course of option is to revert this upstream instead and Cc:stable. Sorry about this. :-/ [1] https://lore.kernel.org/r/20190828094325.GA25048@linux-8ccs
On Wed, Mar 04, 2020 at 10:18:33AM +0100, Jessica Yu wrote: >+++ Lucas De Marchi [03/03/20 22:28 -0800]: >>On Fri, Feb 21, 2020 at 8:53 AM Jessica Yu <jeyu@kernel.org> wrote: >>> >>>depmod -e -E is broken for kernel versions >= 5.4, because a new >>>namespace field was added to Module.symvers. Each line is tab delimited >>>with 5 fields in total. E.g., >>> >>> 0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL >>> >>>When a symbol doesn't have a namespace, then the namespace field is empty: >>> >>> 0xb352177e\tfind_first_bit\t\tvmlinux\tEXPORT_SYMBOL >> >>Why is namespace added in the *middle*? We remember we specifically >>talked about compatibility back when this was added. Why is it not at >>the end so tools that don't know about namespace don't stop working? >> >>Lucas De Marchi > >Sigh, I do remember we had a short discussion upstream back in August >[1] when we were tossing around Module.symvers format preferences. It >is my fault for not having pushed the backwards compatibility option >more instead of thinking it could be patched up in kmod tools. I think >maybe the best course of option is to revert this upstream instead and >Cc:stable. > >Sorry about this. :-/ Sorry, my fault. The discussion went first all around whether we should append the namespace to the symbol name. This we did not do. Then we discussed whether the last values of this line are actually optional and we settled with the format now merged as nobody further objected in the tail end of this discussion: https://lore.kernel.org/linux-modules/20190828101640.GB25048@linux-8ccs/ We could probably move the namespace to the end as the other fields are not optional (at least judging from write_dump() in modpost.c). Cheers, Matthias > >[1] https://lore.kernel.org/r/20190828094325.GA25048@linux-8ccs >
On Wed, Mar 04, 2020 at 10:18:33AM +0100, Jessica Yu wrote: >+++ Lucas De Marchi [03/03/20 22:28 -0800]: >>On Fri, Feb 21, 2020 at 8:53 AM Jessica Yu <jeyu@kernel.org> wrote: >>> >>>depmod -e -E is broken for kernel versions >= 5.4, because a new >>>namespace field was added to Module.symvers. Each line is tab delimited >>>with 5 fields in total. E.g., >>> >>> 0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL >>> >>>When a symbol doesn't have a namespace, then the namespace field is empty: >>> >>> 0xb352177e\tfind_first_bit\t\tvmlinux\tEXPORT_SYMBOL >> >>Why is namespace added in the *middle*? We remember we specifically >>talked about compatibility back when this was added. Why is it not at >>the end so tools that don't know about namespace don't stop working? >> >>Lucas De Marchi > >Sigh, I do remember we had a short discussion upstream back in August >[1] when we were tossing around Module.symvers format preferences. It >is my fault for not having pushed the backwards compatibility option >more instead of thinking it could be patched up in kmod tools. I think >maybe the best course of option is to revert this upstream instead and >Cc:stable. Yeah I didn't follow that series thoroughly as I should. I agree that the best course of action now is to update the format and CC stable. Lucas De Marchi > >Sorry about this. :-/ > >[1] https://lore.kernel.org/r/20190828094325.GA25048@linux-8ccs >
diff --git a/tools/depmod.c b/tools/depmod.c index fbbce10..c5b9612 100644 --- a/tools/depmod.c +++ b/tools/depmod.c @@ -2577,7 +2577,9 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename) { char line[10240]; FILE *fp; - unsigned int linenum = 0; + unsigned int linenum = 0, cnt = 0; + bool uses_namespaces = false; + char *ptr; fp = fopen(filename, "r"); if (fp == NULL) { @@ -2587,7 +2589,26 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename) } DBG("load symvers: %s\n", filename); - /* eg. "0xb352177e\tfind_first_bit\tvmlinux\tEXPORT_SYMBOL" */ + /* + * First, check for >=5.4 Module.symvers format: + * "0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL" + * If there are 5 fields (4 tabs), assume we're using the new format. + */ + fgets(line, sizeof(line), fp); + ptr = line; + while ((ptr = strchr(ptr, '\t')) != NULL) { + cnt++; + ptr++; + } + if (cnt > 3) + uses_namespaces = true; + rewind(fp); + + /* + * eg. "0xb352177e\tfind_first_bit\tvmlinux\tEXPORT_SYMBOL" + * Or if kernel version >=5.4: + * "0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL" + */ while (fgets(line, sizeof(line), fp) != NULL) { const char *ver, *sym, *where; char *verend; @@ -2595,9 +2616,13 @@ static int depmod_load_symvers(struct depmod *depmod, const char *filename) linenum++; - ver = strtok(line, " \t"); - sym = strtok(NULL, " \t"); - where = strtok(NULL, " \t"); + ptr = (char *)line; + ver = strsep(&ptr, "\t"); + sym = strsep(&ptr, "\t"); + if (uses_namespaces) /* skip namespace field */ + strsep(&ptr, "\t"); + where = strsep(&ptr, "\t"); + if (!ver || !sym || !where) continue;
depmod -e -E is broken for kernel versions >= 5.4, because a new namespace field was added to Module.symvers. Each line is tab delimited with 5 fields in total. E.g., 0xb352177e\tfind_first_bit\tnamespace\tvmlinux\tEXPORT_SYMBOL When a symbol doesn't have a namespace, then the namespace field is empty: 0xb352177e\tfind_first_bit\t\tvmlinux\tEXPORT_SYMBOL Fix up depmod_load_symvers() so it finds the crc, symbol, and module ("where") fields correctly. Since there can be empty fields, strsep() is used instead of strtok(), since strtok() cannot handle empty fields (i.e., two delimiters in succession). Signed-off-by: Jessica Yu <jeyu@kernel.org> --- Hi, I was not sure what the best way of determining the symvers format was. I guess counting delimiters isn't the prettiest way, but if anyone has a better idea, let me know. Thanks! tools/depmod.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-)