Linux-Modules Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4)
@ 2020-02-21 16:52 Jessica Yu
  2020-03-03 14:31 ` Jessica Yu
  2020-03-04  6:28 ` Lucas De Marchi
  0 siblings, 2 replies; 6+ messages in thread
From: Jessica Yu @ 2020-02-21 16:52 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Matthias Maennich, linux-modules, Jessica Yu

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(-)

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4)
  2020-02-21 16:52 [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4) Jessica Yu
@ 2020-03-03 14:31 ` Jessica Yu
  2020-03-04  6:28 ` Lucas De Marchi
  1 sibling, 0 replies; 6+ messages in thread
From: Jessica Yu @ 2020-03-03 14:31 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Matthias Maennich, linux-modules

+++ 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4)
  2020-02-21 16:52 [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4) Jessica Yu
  2020-03-03 14:31 ` Jessica Yu
@ 2020-03-04  6:28 ` Lucas De Marchi
  2020-03-04  9:18   ` Jessica Yu
  1 sibling, 1 reply; 6+ messages in thread
From: Lucas De Marchi @ 2020-03-04  6:28 UTC (permalink / raw)
  To: Jessica Yu; +Cc: Matthias Maennich, linux-modules

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4)
  2020-03-04  6:28 ` Lucas De Marchi
@ 2020-03-04  9:18   ` Jessica Yu
  2020-03-04 11:34     ` Matthias Maennich
  2020-03-06  0:10     ` Lucas De Marchi
  0 siblings, 2 replies; 6+ messages in thread
From: Jessica Yu @ 2020-03-04  9:18 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: Matthias Maennich, linux-modules

+++ 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4)
  2020-03-04  9:18   ` Jessica Yu
@ 2020-03-04 11:34     ` Matthias Maennich
  2020-03-06  0:10     ` Lucas De Marchi
  1 sibling, 0 replies; 6+ messages in thread
From: Matthias Maennich @ 2020-03-04 11:34 UTC (permalink / raw)
  To: Jessica Yu; +Cc: Lucas De Marchi, linux-modules

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4)
  2020-03-04  9:18   ` Jessica Yu
  2020-03-04 11:34     ` Matthias Maennich
@ 2020-03-06  0:10     ` Lucas De Marchi
  1 sibling, 0 replies; 6+ messages in thread
From: Lucas De Marchi @ 2020-03-06  0:10 UTC (permalink / raw)
  To: Jessica Yu; +Cc: Lucas De Marchi, Matthias Maennich, linux-modules

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-21 16:52 [PATCH] depmod: account for new namespace field in Module.symvers (for kernel versions >= 5.4) Jessica Yu
2020-03-03 14:31 ` Jessica Yu
2020-03-04  6:28 ` Lucas De Marchi
2020-03-04  9:18   ` Jessica Yu
2020-03-04 11:34     ` Matthias Maennich
2020-03-06  0:10     ` Lucas De Marchi

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org
	public-inbox-index linux-modules

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git