netdev - Re: [PATCH net 2/3] cpsw/netcp: davinci_cpdma: sanitize inter-module API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161216095859.GA16712@khorivan>
Date:   Fri, 16 Dec 2016 11:59:00 +0200
From:   Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Mugunthan V N <mugunthanvnm@...com>,
        Grygorii Strashko <grygorii.strashko@...com>,
        "David S. Miller" <davem@...emloft.net>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>, linux-omap@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 2/3] cpsw/netcp: davinci_cpdma: sanitize inter-module
 API

On Fri, Dec 16, 2016 at 10:19:58AM +0100, Arnd Bergmann wrote:
> The davinci_cpdma module is a helper library that is used by the
> actual device drivers and does nothing by itself, so all its API
> functions need to be exported.
> 
> Four new functions were added recently without an export, so now
> we get a build error:
> 
> ERROR: "cpdma_chan_set_weight" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
> ERROR: "cpdma_chan_get_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
> ERROR: "cpdma_chan_get_min_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
> ERROR: "cpdma_chan_set_rate" [drivers/net/ethernet/ti/ti_cpsw.ko] undefined!
/\
||
A patch correcting this has been applied.
(397c5ad153f0ea62023accb67b3d2b07e5039948)

> 
> This exports those symbols. After taking a closer look, I found two
> more global functions in this file that are not exported and not used
> anywhere, so they can obviously be removed. There is also one function
> that is only used internally in this module, and should be 'static'.
> 
> Fixes: 8f32b90981dc ("net: ethernet: ti: davinci_cpdma: add set rate for a channel")
> Fixes: 0fc6432cc78d ("net: ethernet: ti: davinci_cpdma: add weight function for channels")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/net/ethernet/ti/davinci_cpdma.c | 59 ++++++++++-----------------------
>  drivers/net/ethernet/ti/davinci_cpdma.h |  4 ---
>  2 files changed, 17 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
> index 36518fc5c7cc..b9d40f0cdf6c 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.c
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.c
> @@ -628,6 +628,23 @@ int cpdma_ctlr_destroy(struct cpdma_ctlr *ctlr)
>  }
>  EXPORT_SYMBOL_GPL(cpdma_ctlr_destroy);
>  
> +static int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&chan->lock, flags);
> +	if (chan->state != CPDMA_STATE_ACTIVE) {
> +		spin_unlock_irqrestore(&chan->lock, flags);
> +		return -EINVAL;
> +	}
> +
> +	dma_reg_write(chan->ctlr, enable ? chan->int_set : chan->int_clear,
> +		      chan->mask);
> +	spin_unlock_irqrestore(&chan->lock, flags);
> +
> +	return 0;
> +}
> +
>  int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable)
>  {
>  	unsigned long flags;
> @@ -1274,46 +1291,4 @@ int cpdma_chan_stop(struct cpdma_chan *chan)
>  }
>  EXPORT_SYMBOL_GPL(cpdma_chan_stop);
>  
> -int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable)
> -{
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&chan->lock, flags);
> -	if (chan->state != CPDMA_STATE_ACTIVE) {
> -		spin_unlock_irqrestore(&chan->lock, flags);
> -		return -EINVAL;
> -	}
> -
> -	dma_reg_write(chan->ctlr, enable ? chan->int_set : chan->int_clear,
> -		      chan->mask);
> -	spin_unlock_irqrestore(&chan->lock, flags);
> -
> -	return 0;
> -}
> -
> -int cpdma_control_get(struct cpdma_ctlr *ctlr, int control)
> -{
> -	unsigned long flags;
> -	int ret;
> -
> -	spin_lock_irqsave(&ctlr->lock, flags);
> -	ret = _cpdma_control_get(ctlr, control);
> -	spin_unlock_irqrestore(&ctlr->lock, flags);
> -
> -	return ret;
> -}
> -
> -int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value)
> -{
> -	unsigned long flags;
> -	int ret;
> -
> -	spin_lock_irqsave(&ctlr->lock, flags);
> -	ret = _cpdma_control_set(ctlr, control, value);
> -	spin_unlock_irqrestore(&ctlr->lock, flags);
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(cpdma_control_set);
> -
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/ti/davinci_cpdma.h b/drivers/net/ethernet/ti/davinci_cpdma.h
> index 4a167db2abab..36d0a09a3d44 100644
> --- a/drivers/net/ethernet/ti/davinci_cpdma.h
> +++ b/drivers/net/ethernet/ti/davinci_cpdma.h
> @@ -87,7 +87,6 @@ int cpdma_chan_process(struct cpdma_chan *chan, int quota);
>  
>  int cpdma_ctlr_int_ctrl(struct cpdma_ctlr *ctlr, bool enable);
>  void cpdma_ctlr_eoi(struct cpdma_ctlr *ctlr, u32 value);
> -int cpdma_chan_int_ctrl(struct cpdma_chan *chan, bool enable);
>  u32 cpdma_ctrl_rxchs_state(struct cpdma_ctlr *ctlr);
>  u32 cpdma_ctrl_txchs_state(struct cpdma_ctlr *ctlr);
>  bool cpdma_check_free_tx_desc(struct cpdma_chan *chan);
> @@ -111,7 +110,4 @@ enum cpdma_control {
>  	CPDMA_RX_BUFFER_OFFSET,		/* read-write */
>  };
>  
> -int cpdma_control_get(struct cpdma_ctlr *ctlr, int control);
> -int cpdma_control_set(struct cpdma_ctlr *ctlr, int control, int value);
> -
>  #endif
> -- 
> 2.9.0
>