linux-kernel - Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use kernel container

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: <20170614101853.GA8107@kroah.com>
Date:   Wed, 14 Jun 2017 12:18:53 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Joe Perches <joe@...ches.com>
Cc:     linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
        Hans de Goede <hdegoede@...hat.com>,
        Larry Finger <Larry.Finger@...inger.net>
Subject: Re: [PATCH 1/2] staging: rtl8723bs: Convert LIST_CONTAINOR to use
 kernel container_of

On Tue, Jun 13, 2017 at 02:12:56PM -0700, Joe Perches wrote:
> These are similar macros so use the normal kernel one.
> 
> As well, there are odd games being played with casting a plist to
> a union recv_frame by using LIST_CONTAINOR.  Just use a direct cast
> to union recv_frame instead.
> 
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_recv.c               | 14 +++++++-------
>  drivers/staging/rtl8723bs/core/rtw_sta_mgt.c            |  2 +-
>  drivers/staging/rtl8723bs/include/osdep_service_linux.h |  2 +-
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 695a5c958c80..e1d6d0a4b115 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -129,7 +129,7 @@ union recv_frame *_rtw_alloc_recvframe(struct __queue *pfree_recv_queue)
>  
>  		plist = get_next(phead);
>  
> -		precvframe = LIST_CONTAINOR(plist, union recv_frame, u);
> +		precvframe = (union recv_frame *)plist;

No, you are "assuming" that the list_head is going to stay the first
object of this structure, and what if it isn't?

Just use container_of, that way at least you get the type safeness of
the call, and if something changes in the future, you don't instantly
break the code everywhere without knowing it.

thanks,

greg k-h