| Message ID | 20191112111600.18719-2-o.rempel@pengutronix.de |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | can: j1939: fix multiple issues found by syzbot | expand |
Hello, On Tue, Nov 12, 2019 at 12:15:52PM +0100, Oleksij Rempel wrote: > In j1939 we need our own struct sock::sk_destruct callback. Export the > generic af_can can_sock_destruct() that allows us to chain-call it. > > Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > include/linux/can/core.h | 1 + > net/can/af_can.c | 3 ++- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/include/linux/can/core.h b/include/linux/can/core.h > index 8339071ab08b..e20a0cd09ba5 100644 > --- a/include/linux/can/core.h > +++ b/include/linux/can/core.h > @@ -65,5 +65,6 @@ extern void can_rx_unregister(struct net *net, struct net_device *dev, > void *data); > > extern int can_send(struct sk_buff *skb, int loop); > +void can_sock_destruct(struct sock *sk); > > #endif /* !_CAN_CORE_H */ > diff --git a/net/can/af_can.c b/net/can/af_can.c > index 5518a7d9eed9..128d37a4c2e0 100644 > --- a/net/can/af_can.c > +++ b/net/can/af_can.c > @@ -86,11 +86,12 @@ static atomic_t skbcounter = ATOMIC_INIT(0); > > /* af_can socket functions */ > > -static void can_sock_destruct(struct sock *sk) > +void can_sock_destruct(struct sock *sk) > { > skb_queue_purge(&sk->sk_receive_queue); > skb_queue_purge(&sk->sk_error_queue); > } > +EXPORT_SYMBOL(can_sock_destruct); If the users are only expected to be another can module, it might make sense to use a namespace here?! Best regards Uwe
On 11/12/19 12:37 PM, Uwe Kleine-König wrote: > Hello, > > On Tue, Nov 12, 2019 at 12:15:52PM +0100, Oleksij Rempel wrote: >> In j1939 we need our own struct sock::sk_destruct callback. Export the >> generic af_can can_sock_destruct() that allows us to chain-call it. >> >> Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> >> --- >> include/linux/can/core.h | 1 + >> net/can/af_can.c | 3 ++- >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/can/core.h b/include/linux/can/core.h >> index 8339071ab08b..e20a0cd09ba5 100644 >> --- a/include/linux/can/core.h >> +++ b/include/linux/can/core.h >> @@ -65,5 +65,6 @@ extern void can_rx_unregister(struct net *net, struct net_device *dev, >> void *data); >> >> extern int can_send(struct sk_buff *skb, int loop); >> +void can_sock_destruct(struct sock *sk); >> >> #endif /* !_CAN_CORE_H */ >> diff --git a/net/can/af_can.c b/net/can/af_can.c >> index 5518a7d9eed9..128d37a4c2e0 100644 >> --- a/net/can/af_can.c >> +++ b/net/can/af_can.c >> @@ -86,11 +86,12 @@ static atomic_t skbcounter = ATOMIC_INIT(0); >> >> /* af_can socket functions */ >> >> -static void can_sock_destruct(struct sock *sk) >> +void can_sock_destruct(struct sock *sk) >> { >> skb_queue_purge(&sk->sk_receive_queue); >> skb_queue_purge(&sk->sk_error_queue); >> } >> +EXPORT_SYMBOL(can_sock_destruct); > > If the users are only expected to be another can module, it might make > sense to use a namespace here?! How? Marc
Hello Marc, On Tue, Nov 12, 2019 at 12:39:27PM +0100, Marc Kleine-Budde wrote: > On 11/12/19 12:37 PM, Uwe Kleine-König wrote: > > On Tue, Nov 12, 2019 at 12:15:52PM +0100, Oleksij Rempel wrote: > >> +EXPORT_SYMBOL(can_sock_destruct); > > > > If the users are only expected to be another can module, it might make > > sense to use a namespace here?! > > How? Use EXPORT_SYMBOL_NS(can_sock_destruct, CAN) instead of the plain EXPORT_SYMBOL, and near the declaration of can_sock_destruct or in the source that makes use of the symbol add: MODULE_IMPORT_NS(CAN); See https://lwn.net/Articles/760045/ for some details. Best regards Uwe
On 12/11/2019 12.45, Uwe Kleine-König wrote: > Hello Marc, > > On Tue, Nov 12, 2019 at 12:39:27PM +0100, Marc Kleine-Budde wrote: >> On 11/12/19 12:37 PM, Uwe Kleine-König wrote: >>> On Tue, Nov 12, 2019 at 12:15:52PM +0100, Oleksij Rempel wrote: >>>> +EXPORT_SYMBOL(can_sock_destruct); >>> >>> If the users are only expected to be another can module, it might make >>> sense to use a namespace here?! >> >> How? > > Use > > EXPORT_SYMBOL_NS(can_sock_destruct, CAN) > > instead of the plain EXPORT_SYMBOL, and near the declaration of > can_sock_destruct or in the source that makes use of the symbol add: > > MODULE_IMPORT_NS(CAN); > > See https://lwn.net/Articles/760045/ for some details. Looks nice! Good idea! But I would tend to introduce the symbol namespaces for this and the other (existing) symbols via can-next and not within this patch set that addresses the j1939 fixes. Best regards, Oliver
On 11/12/19 7:24 PM, Oliver Hartkopp wrote: > > > On 12/11/2019 12.45, Uwe Kleine-König wrote: >> Hello Marc, >> >> On Tue, Nov 12, 2019 at 12:39:27PM +0100, Marc Kleine-Budde wrote: >>> On 11/12/19 12:37 PM, Uwe Kleine-König wrote: >>>> On Tue, Nov 12, 2019 at 12:15:52PM +0100, Oleksij Rempel wrote: >>>>> +EXPORT_SYMBOL(can_sock_destruct); >>>> >>>> If the users are only expected to be another can module, it might make >>>> sense to use a namespace here?! >>> >>> How? >> >> Use >> >> EXPORT_SYMBOL_NS(can_sock_destruct, CAN) >> >> instead of the plain EXPORT_SYMBOL, and near the declaration of >> can_sock_destruct or in the source that makes use of the symbol add: >> >> MODULE_IMPORT_NS(CAN); >> >> See https://lwn.net/Articles/760045/ for some details. > > Looks nice! Good idea! > > But I would tend to introduce the symbol namespaces for this and the > other (existing) symbols via can-next and not within this patch set that > addresses the j1939 fixes. So I should take this series as is? And the CAN namespace is introduced later? Marc
On 12/11/2019 21.10, Marc Kleine-Budde wrote: > So I should take this series as is? > > And the CAN namespace is introduced later? Aeh - yes. Sorry for my late reply. You already did it right. Best, Oliver
diff --git a/include/linux/can/core.h b/include/linux/can/core.h index 8339071ab08b..e20a0cd09ba5 100644 --- a/include/linux/can/core.h +++ b/include/linux/can/core.h @@ -65,5 +65,6 @@ extern void can_rx_unregister(struct net *net, struct net_device *dev, void *data); extern int can_send(struct sk_buff *skb, int loop); +void can_sock_destruct(struct sock *sk); #endif /* !_CAN_CORE_H */ diff --git a/net/can/af_can.c b/net/can/af_can.c index 5518a7d9eed9..128d37a4c2e0 100644 --- a/net/can/af_can.c +++ b/net/can/af_can.c @@ -86,11 +86,12 @@ static atomic_t skbcounter = ATOMIC_INIT(0); /* af_can socket functions */ -static void can_sock_destruct(struct sock *sk) +void can_sock_destruct(struct sock *sk) { skb_queue_purge(&sk->sk_receive_queue); skb_queue_purge(&sk->sk_error_queue); } +EXPORT_SYMBOL(can_sock_destruct); static const struct can_proto *can_get_proto(int protocol) {
In j1939 we need our own struct sock::sk_destruct callback. Export the generic af_can can_sock_destruct() that allows us to chain-call it. Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol") Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- include/linux/can/core.h | 1 + net/can/af_can.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)