Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
On Thu, Sep 05, 2019 at 08:25:32PM +0900, Masahiro Yamada wrote: > On Thu, Sep 5, 2019 at 8:16 PM Jessica Yu wrote: > > > > +++ Arnd Bergmann [05/09/19 12:52 +0200]: > > >On Thu, Sep 5, 2019 at 12:41 PM Jessica Yu wrote: > > >> +++ Matthew Dharm [04/09/19 09:16 -0700]: > > >> >On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: > > > > > >> >HOWEVER, I have one question: If these patches are included, and > > >> >someone wants to introduce a bit of code which needs to use two > > >> >symbols from different namespaces but with the same name, can that be > > >> >done? That is, if driver A has symbol 'foo' and driver B has symbol > > >> >'foo' (both in their respective namespaces), and driver C wants to use > > >> >A.foo and B.foo, can that be supported? > > >> > > >> As of now, we currently don't support this - modpost will warn if a > > >> symbol is exported more than once (across modules + vmlinux), and the > > >> module loader currently assumes exported symbol names are unique. Do > > >> you have a concrete use case? If there is a strong need for this, I > > >> don't think it'd be too hard to implement. > > > > > >I think what would prevent this from working in general is that having > > >two modules with the same exported symbol in different namespaces > > >won't link if you try to build both modules into the kernel itself. > > > > > > Arnd > > > > Ah yeah, you are right. I only tried building an identically named > > exported symbol in a module and in the kernel, and there I got away > > with a modpost warning. But this breaks when building the module into > > the kernel, so I guess this is out of the question. > > > > Thanks, > > > > Jessica > > > > > The cover letter starts with > "As of Linux 5.3-rc7, there are 31207 [1] exported symbols in the kernel". > > Whether or not we apply this patch set, > we will have to carefully maintain them > so that 31207 symbols are unique, anyway. > (And, we can do this with allmodconfig + modpost) > > So, what is the point of the namespace, > where it does not loosen the scope of uniqueness? It does not "loosen" anything. The problem is, we have 31207 exported symbols and no one has any idea how they are "clustered" :) Ideally we would have separate namespaces where we could "keep" others from accessing symbols that are required to be global, but we do not want anyone else to use them. But we have C, and we need to live with that. This is the "next best thing". This provides a namespace and a marking by the user of that exported symbol that they really do know what they are doing. The USB storage exports are a concrete example of this in this patchset. Those symbols are to only be used by other USB storage drivers, and they are marked that way here. This also gives us the ability to start to actually understand what our different exported symbols are for. Right now the USB core exports a ton of stuff, but who is expected to use what? Right now any USB driver can use any of those functions and no one notices. After this patchset is merged, I will start to "carve up" the USB exports into different "namespaces". One example will be the functions only for use by USB host controller drivers. Then if someone submits a new driver that says "use the USB host controller driver namespace", and that driver is _not_ a USB host controller driver, it's a huge red flag that something really isn't correct here. Some other USB functions will probably get marked "USB core only" or some such thing, and then to use them you have to say you are importing from the "usb core only" which again, is a huge flag that you are doing something wrong. So on it's own, this patchset doesn't do much. But it gives us the building blocks on which to start to make sense of those 31000+ different exported symbols. As an aside, this is something that I know I and others have been wanting to see happen for 10+ years now, and I'm very happy to see it show up in a workable form as we have known this is a issue for a very long time. Hope this helps explain things better, greg k-h
Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
On Thu, Sep 5, 2019 at 8:16 PM Jessica Yu wrote: > > +++ Arnd Bergmann [05/09/19 12:52 +0200]: > >On Thu, Sep 5, 2019 at 12:41 PM Jessica Yu wrote: > >> +++ Matthew Dharm [04/09/19 09:16 -0700]: > >> >On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: > > > >> >HOWEVER, I have one question: If these patches are included, and > >> >someone wants to introduce a bit of code which needs to use two > >> >symbols from different namespaces but with the same name, can that be > >> >done? That is, if driver A has symbol 'foo' and driver B has symbol > >> >'foo' (both in their respective namespaces), and driver C wants to use > >> >A.foo and B.foo, can that be supported? > >> > >> As of now, we currently don't support this - modpost will warn if a > >> symbol is exported more than once (across modules + vmlinux), and the > >> module loader currently assumes exported symbol names are unique. Do > >> you have a concrete use case? If there is a strong need for this, I > >> don't think it'd be too hard to implement. > > > >I think what would prevent this from working in general is that having > >two modules with the same exported symbol in different namespaces > >won't link if you try to build both modules into the kernel itself. > > > > Arnd > > Ah yeah, you are right. I only tried building an identically named > exported symbol in a module and in the kernel, and there I got away > with a modpost warning. But this breaks when building the module into > the kernel, so I guess this is out of the question. > > Thanks, > > Jessica > The cover letter starts with "As of Linux 5.3-rc7, there are 31207 [1] exported symbols in the kernel". Whether or not we apply this patch set, we will have to carefully maintain them so that 31207 symbols are unique, anyway. (And, we can do this with allmodconfig + modpost) So, what is the point of the namespace, where it does not loosen the scope of uniqueness? -- Best Regards Masahiro Yamada
Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
On Thu, Sep 05, 2019 at 12:41:47PM +0200, Jessica Yu wrote: +++ Matthew Dharm [04/09/19 09:16 -0700]: On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: Note that I don't object to the patch set in general. There may be symbols which only need be exported in the context of a single subsystem or even driver (if a driver consists of more than one module). For example, a mfd driver may export symbols which should only be called by its client drivers. In such a situation, it may well be beneficial to limit the use of exported symbols. I can appreciate this benefit. I am not sure what good that does in practice (if I understand correctly, a driver only has to declare that it wants to use a restricted use symbol if it wants to use it), but that is a different question. I think this question implies that you are coming from the perspective of "security" or wanting to restrict access to the underlying functions, rather than wanting to clean-up the way symbols are handled for manageability / maintainability purposes (which is the goal, as I understand it). The goal of this patch set is to introduce structure into the exported surface that goes beyond naming conventions like 'usb_*'. So, it is rather about maintainability then security. In particular, creating the visibility of which parts of the kernel use which other parts, might help to find cases where suboptimal choices were made. Maybe already during development/review. As Guenter correctly noted, a module is able to declare that it wants to use a namespace. One idea that came up earlier was to maybe restrict the namespaces that can actually be imported by modules. But I would see anything in that direction as beyond the scope of this series. A nice side effect of having to declare the usage is that it shows up in modinfo and module users can reason about how the module interacts with the rest of the kernel. HOWEVER, I have one question: If these patches are included, and someone wants to introduce a bit of code which needs to use two symbols from different namespaces but with the same name, can that be done? That is, if driver A has symbol 'foo' and driver B has symbol 'foo' (both in their respective namespaces), and driver C wants to use A.foo and B.foo, can that be supported? As of now, we currently don't support this - modpost will warn if a symbol is exported more than once (across modules + vmlinux), and the module loader currently assumes exported symbol names are unique. Do you have a concrete use case? If there is a strong need for this, I don't think it'd be too hard to implement. The implementation does not change the fact that symbol names need to be unique. As Arnd just mentioned in the other thread: the linker will already fail if two builtin symbols use the same name. It is rather a tag attached to the symbol. Cheers, Matthias
Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
+++ Arnd Bergmann [05/09/19 12:52 +0200]: On Thu, Sep 5, 2019 at 12:41 PM Jessica Yu wrote: +++ Matthew Dharm [04/09/19 09:16 -0700]: >On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: >HOWEVER, I have one question: If these patches are included, and >someone wants to introduce a bit of code which needs to use two >symbols from different namespaces but with the same name, can that be >done? That is, if driver A has symbol 'foo' and driver B has symbol >'foo' (both in their respective namespaces), and driver C wants to use >A.foo and B.foo, can that be supported? As of now, we currently don't support this - modpost will warn if a symbol is exported more than once (across modules + vmlinux), and the module loader currently assumes exported symbol names are unique. Do you have a concrete use case? If there is a strong need for this, I don't think it'd be too hard to implement. I think what would prevent this from working in general is that having two modules with the same exported symbol in different namespaces won't link if you try to build both modules into the kernel itself. Arnd Ah yeah, you are right. I only tried building an identically named exported symbol in a module and in the kernel, and there I got away with a modpost warning. But this breaks when building the module into the kernel, so I guess this is out of the question. Thanks, Jessica
Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
On Thu, Sep 5, 2019 at 12:41 PM Jessica Yu wrote: > +++ Matthew Dharm [04/09/19 09:16 -0700]: > >On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: > >HOWEVER, I have one question: If these patches are included, and > >someone wants to introduce a bit of code which needs to use two > >symbols from different namespaces but with the same name, can that be > >done? That is, if driver A has symbol 'foo' and driver B has symbol > >'foo' (both in their respective namespaces), and driver C wants to use > >A.foo and B.foo, can that be supported? > > As of now, we currently don't support this - modpost will warn if a > symbol is exported more than once (across modules + vmlinux), and the > module loader currently assumes exported symbol names are unique. Do > you have a concrete use case? If there is a strong need for this, I > don't think it'd be too hard to implement. I think what would prevent this from working in general is that having two modules with the same exported symbol in different namespaces won't link if you try to build both modules into the kernel itself. Arnd
Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
+++ Matthew Dharm [04/09/19 09:16 -0700]: On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: Note that I don't object to the patch set in general. There may be symbols which only need be exported in the context of a single subsystem or even driver (if a driver consists of more than one module). For example, a mfd driver may export symbols which should only be called by its client drivers. In such a situation, it may well be beneficial to limit the use of exported symbols. I can appreciate this benefit. I am not sure what good that does in practice (if I understand correctly, a driver only has to declare that it wants to use a restricted use symbol if it wants to use it), but that is a different question. I think this question implies that you are coming from the perspective of "security" or wanting to restrict access to the underlying functions, rather than wanting to clean-up the way symbols are handled for manageability / maintainability purposes (which is the goal, as I understand it). HOWEVER, I have one question: If these patches are included, and someone wants to introduce a bit of code which needs to use two symbols from different namespaces but with the same name, can that be done? That is, if driver A has symbol 'foo' and driver B has symbol 'foo' (both in their respective namespaces), and driver C wants to use A.foo and B.foo, can that be supported? As of now, we currently don't support this - modpost will warn if a symbol is exported more than once (across modules + vmlinux), and the module loader currently assumes exported symbol names are unique. Do you have a concrete use case? If there is a strong need for this, I don't think it'd be too hard to implement. Thanks, Jessica
Re: [usb-storage] Re: [PATCH v4 12/12] RFC: watchdog: export core symbols in WATCHDOG_CORE namespace
On Wed, Sep 4, 2019 at 5:12 AM Guenter Roeck wrote: > > Note that I don't object to the patch set in general. There may be symbols > which only need be exported in the context of a single subsystem or even > driver (if a driver consists of more than one module). For example, a mfd > driver may export symbols which should only be called by its client drivers. > In such a situation, it may well be beneficial to limit the use of exported > symbols. I can appreciate this benefit. > I am not sure what good that does in practice (if I understand correctly, > a driver only has to declare that it wants to use a restricted use symbol > if it wants to use it), but that is a different question. I think this question implies that you are coming from the perspective of "security" or wanting to restrict access to the underlying functions, rather than wanting to clean-up the way symbols are handled for manageability / maintainability purposes (which is the goal, as I understand it). HOWEVER, I have one question: If these patches are included, and someone wants to introduce a bit of code which needs to use two symbols from different namespaces but with the same name, can that be done? That is, if driver A has symbol 'foo' and driver B has symbol 'foo' (both in their respective namespaces), and driver C wants to use A.foo and B.foo, can that be supported? Matt -- Matthew Dharm Former Maintainer, USB Mass Storage driver for Linux
