[5.3] ucc_geth: Fix to avoid IS_ERR_VALUE abuses and dead code on 64bit systems.

David Laight David.Laight at ACULAB.COM
Tue Aug 9 00:49:11 AEST 2016


From: netdev-owner at vger.kernel.org [mailto:netdev-owner at vger.kernel.org] On Behalf Of Arvind Yadav
> IS_ERR_VALUE() assumes that parameter is an unsigned long.
> It can not be used to check if 'unsigned int' is passed insted.
> Which tends to reflect an error.
> In 64bit architectures sizeof (int) == 4 && sizeof (long) == 8.
> IS_ERR_VALUE(x) is ((x) >= (unsigned long)-4095).
> IS_ERR_VALUE() of 'unsigned int' is always false because the 32bit
> value is zero extended to 64 bits.

You are being far too wordy above, and definitely below.

> 
> Now problem in Freescale QEGigabit Ethernet-:
>                                  drivers/net/ethernet/freescale/ucc_geth.c
> 
...
>          qe_muram_addr(init_enet_pram_offset);
> 
> qe_muram_alloc (a.k.a. cpm_muram_alloc) returns unsigned long.
> Return value store in a u32 (init_enet_offset, exf_glbl_param_offset,
> rx_glbl_pram_offset, tx_glbl_pram_offset, send_q_mem_reg_offset,
> thread_dat_tx_offset, thread_dat_rx_offset, scheduler_offset,
> tx_fw_statistics_pram_offset, rx_fw_statistics_pram_offset,
> rx_irq_coalescing_tbl_offset, rx_bd_qs_tbl_offset, tx_bd_ring_offset,
> init_enet_pram_offset and rx_bd_ring_offset).

Inpenetrable...

> If qe_muram_alloc will return any error, Then IS_ERR_VALUE will always
> return 0. it'll not call ucc_fast_free for any failure. Inside 'if code'
> will be a dead code on 64bit. Even qe_muram_addr will return wrong
> virtual address. Which can cause an error.
> 
>  kfree((void *)ugeth->tx_bd_ring_offset[i]);

Erm, kfree() isn't the right function for things allocated by qe_muram_alloc().

I still thing you need to stop this code using IS_ERR_VALUE() at all.

	David



More information about the Linuxppc-dev mailing list