[PATCH] lib80211: Use ERR_CAST() to return

[PATCH] lib80211: Use ERR_CAST() to return

[Rohit Chavan]

Using ERR_CAST() is more reasonable and safer, When it is necessary
to convert the type of an error pointer and return it.

Signed-off-by: Rohit Chavan <roheetchavan@gmail.com>
---
 net/wireless/lib80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/lib80211.c b/net/wireless/lib80211.c
index d66a913027e0..87c0e09aa676 100644
--- a/net/wireless/lib80211.c
+++ b/net/wireless/lib80211.c
@@ -227,7 +227,7 @@ EXPORT_SYMBOL(lib80211_get_crypto_ops);
 
 static void *lib80211_crypt_null_init(int keyidx)
 {
-	return (void *)1;
+	return ERR_CAST(1);
 }
 
 static void lib80211_crypt_null_deinit(void *priv)
-- 
2.34.1

Re: [PATCH] lib80211: Use ERR_CAST() to return

[Johannes Berg]

On Fri, 2024-09-06 at 17:14 +0530, Rohit Chavan wrote:
> Using ERR_CAST() is more reasonable and safer, When it is necessary
> to convert the type of an error pointer and return it.
> 

What? Why? What's the point?

johannes

Re: [PATCH] lib80211: Use ERR_CAST() to return

[Simon Horman]

On Fri, Sep 06, 2024 at 05:14:55PM +0530, Rohit Chavan wrote:
> Using ERR_CAST() is more reasonable and safer, When it is necessary
> to convert the type of an error pointer and return it.
> 
> Signed-off-by: Rohit Chavan <roheetchavan@gmail.com>
> ---
>  net/wireless/lib80211.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/wireless/lib80211.c b/net/wireless/lib80211.c
> index d66a913027e0..87c0e09aa676 100644
> --- a/net/wireless/lib80211.c
> +++ b/net/wireless/lib80211.c
> @@ -227,7 +227,7 @@ EXPORT_SYMBOL(lib80211_get_crypto_ops);
>  
>  static void *lib80211_crypt_null_init(int keyidx)
>  {
> -	return (void *)1;
> +	return ERR_CAST(1);

This seems wrong to me. ERR_CAST is designed to cast a pointer
to an error pointer. But 1 is an integer, not a pointer.

>  }
>  
>  static void lib80211_crypt_null_deinit(void *priv)
> -- 
> 2.34.1
> 
> 

Re: [PATCH] lib80211: Use ERR_CAST() to return

[kernel test robot]

Hi Rohit,

kernel test robot noticed the following build errors:

[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rohit-Chavan/lib80211-Use-ERR_CAST-to-return/20240906-194721
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20240906114455.730559-1-roheetchavan%40gmail.com
patch subject: [PATCH] lib80211: Use ERR_CAST() to return
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240907/202409071405.Ze3koraa-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/202409071405.Ze3koraa-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409071405.Ze3koraa-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/wireless/lib80211.c: In function 'lib80211_crypt_null_init':
>> net/wireless/lib80211.c:230:25: error: passing argument 1 of 'ERR_CAST' makes pointer from integer without a cast [-Wint-conversion]
     230 |         return ERR_CAST(1);
         |                         ^
         |                         |
         |                         int
   In file included from include/linux/string.h:10,
                    from include/linux/bitmap.h:13,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:63,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/umh.h:4,
                    from include/linux/kmod.h:9,
                    from include/linux/module.h:17,
                    from net/wireless/lib80211.c:19:
   include/linux/err.h:82:64: note: expected 'const void *' but argument is of type 'int'
      82 | static inline void * __must_check ERR_CAST(__force const void *ptr)
         |                                                    ~~~~~~~~~~~~^~~


vim +/ERR_CAST +230 net/wireless/lib80211.c

   227	
   228	static void *lib80211_crypt_null_init(int keyidx)
   229	{
 > 230		return ERR_CAST(1);
   231	}
   232	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] lib80211: Use ERR_CAST() to return

[kernel test robot]

Hi Rohit,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rohit-Chavan/lib80211-Use-ERR_CAST-to-return/20240906-194721
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20240906114455.730559-1-roheetchavan%40gmail.com
patch subject: [PATCH] lib80211: Use ERR_CAST() to return
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20240907/202409071509.vfmUeHx0-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/202409071509.vfmUeHx0-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409071509.vfmUeHx0-lkp@intel.com/

All warnings (new ones prefixed by >>):

   net/wireless/lib80211.c: In function 'lib80211_crypt_null_init':
>> net/wireless/lib80211.c:230:25: warning: passing argument 1 of 'ERR_CAST' makes pointer from integer without a cast [-Wint-conversion]
     230 |         return ERR_CAST(1);
         |                         ^
         |                         |
         |                         int
   In file included from include/linux/string.h:10,
                    from include/linux/bitmap.h:13,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/spinlock.h:63,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:7,
                    from include/linux/umh.h:4,
                    from include/linux/kmod.h:9,
                    from include/linux/module.h:17,
                    from net/wireless/lib80211.c:19:
   include/linux/err.h:82:64: note: expected 'const void *' but argument is of type 'int'
      82 | static inline void * __must_check ERR_CAST(__force const void *ptr)
         |                                                    ~~~~~~~~~~~~^~~


vim +/ERR_CAST +230 net/wireless/lib80211.c

   227	
   228	static void *lib80211_crypt_null_init(int keyidx)
   229	{
 > 230		return ERR_CAST(1);
   231	}
   232	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] lib80211: Use ERR_CAST() to return

[Al Viro]

On Fri, Sep 06, 2024 at 05:14:55PM +0530, Rohit Chavan wrote:
> Using ERR_CAST() is more reasonable and safer, When it is necessary
> to convert the type of an error pointer and return it.

Why is it safer?

Please, explain the reasoning that had lead you to this conclusion.
This is a serious request, BTW.

From my reading of that code (and its well-hidden callers - AFAICS, those
are
drivers/net/wireless/intel/ipw2x00/libipw_wx.c:382:                     new_crypt->priv = new_crypt->ops->init(key);
drivers/net/wireless/intel/ipw2x00/libipw_wx.c:611:                     new_crypt->priv = new_crypt->ops->init(idx);
drivers/staging/rtl8192e/rtllib_wx.c:347:                       new_crypt->priv = new_crypt->ops->init(key);
drivers/staging/rtl8192e/rtllib_wx.c:570:                       new_crypt->priv = new_crypt->ops->init(idx);
) what we have is unfortunate calling conventions for that ->init() method.

It reports failures by returning NULL; that's not a good idea for
something that is supposed to allocate a crypto-algorithm-specific
object, since anything that does *NOT* need any allocations at all
can't just return NULL.

This "(void *)1" is basically "let me invent some non-NULL pointer,
I won't be using it at all".  Yes, it's a kludge; there are more idiomatic
ways, but that really ought to be discussed with maintainers, especially
since there are only two places using that sucker in the first place,
one of them being in staging...

In any case, that (void *)1 is _not_ something you want to express as
ERR_CAST() (or ERR_PTR(), for that matter); that would only make the damn
thing harder for readers.

Speaking of making things harder for readers, near the 4th of these callers
there's something weird:
        ops = lib80211_get_crypto_ops(alg);
        if (!ops) {
                char tempbuf[100];

                memset(tempbuf, 0x00, 100);
                sprintf(tempbuf, "%s", module);
                request_module("%s", tempbuf);
                ops = lib80211_get_crypto_ops(alg);
        }

What the hell is going on there?  module is one of the "rtllib_crypt_wep",
"rtllib_crypt_tkip" and "rtllib_crypt_ccmp"...  Looks like a very odd
obfuscation; other callers also come with calls of request_module(),
but those don't do that insane dance...

Re: [PATCH] lib80211: Use ERR_CAST() to return

[kernel test robot]

Hi Rohit,

kernel test robot noticed the following build errors:

[auto build test ERROR on wireless-next/main]
[also build test ERROR on wireless/main linus/master v6.11-rc6 next-20240906]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rohit-Chavan/lib80211-Use-ERR_CAST-to-return/20240906-194721
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/20240906114455.730559-1-roheetchavan%40gmail.com
patch subject: [PATCH] lib80211: Use ERR_CAST() to return
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240908/202409080536.stm6x1AU-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 05f5a91d00b02f4369f46d076411c700755ae041)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409080536.stm6x1AU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409080536.stm6x1AU-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/wireless/lib80211.c:21:
   In file included from include/linux/ieee80211.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:10:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from net/wireless/lib80211.c:21:
   In file included from include/linux/ieee80211.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from net/wireless/lib80211.c:21:
   In file included from include/linux/ieee80211.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from net/wireless/lib80211.c:21:
   In file included from include/linux/ieee80211.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> net/wireless/lib80211.c:230:18: error: incompatible integer to pointer conversion passing 'int' to parameter of type 'const void *' [-Wint-conversion]
     230 |         return ERR_CAST(1);
         |                         ^
   include/linux/err.h:82:64: note: passing argument to parameter 'ptr' here
      82 | static inline void * __must_check ERR_CAST(__force const void *ptr)
         |                                                                ^
   7 warnings and 1 error generated.


vim +230 net/wireless/lib80211.c

   227	
   228	static void *lib80211_crypt_null_init(int keyidx)
   229	{
 > 230		return ERR_CAST(1);
   231	}
   232	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki