Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 20 Apr 2022 14:55:34 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Pavel Skripkin <paskripkin@gmail.com>
Cc:     Jaehee Park <jhpark1013@gmail.com>, Larry.Finger@lwfinger.net,
        phil@philpotter.co.uk, gregkh@linuxfoundation.org,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
        outreachy@lists.linux.dev
Subject: Re: [PATCH v3 1/7] staging: r8188eu: remove unused member
 free_bss_buf
Message-ID: <20220420115534.GF2951@kadam>
References: <cover.1650392020.git.jhpark1013@gmail.com>
 <3c5a510938ac395a13bb09c0de1868cce8ca3dd8.1650392020.git.jhpark1013@gmail.com>
 <32587233-0ff6-ed0f-b873-cd4f797005a9@gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <32587233-0ff6-ed0f-b873-cd4f797005a9@gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?J9OWAPBXG5mGUPWcD0s4sB4U7k2XZau/9a48x1dNgpSm1NAy1XS+647dj8UG?=
 =?us-ascii?Q?N8Ec6Q5TiPvKjFNEjHBMazAW64p09Ro9wu4GVHm4h6ixPhPdG+3hKiFsWxJ0?=
 =?us-ascii?Q?J+beHXwGdW6ruwry2JPXSCAjuKZTUKLFg7rlmtc5JYa9DzgjFRR7Ve2paLiQ?=
 =?us-ascii?Q?UiZaocJ7LtlRztinZWse9yQnVTCPshXFHehCY5cAMWERYUiSPBjYTHt9DBSv?=
 =?us-ascii?Q?2LoKAExaUcQ2ApEdOnstOuEHqReqYCRhEOrAxKtWO+Amy4hXdeUv8NFqzsr5?=
 =?us-ascii?Q?QeUlIylAmkrouDXVhzMGKqDT8+dFVaWOFwWU62WeJ4b39jFAJLH+k0IeG3ai?=
 =?us-ascii?Q?8rh2UKXN0KVa8Tm+sDO06UezW+TZxKKMH9L0llNkEhWmcSiZumkXlNVm+ILh?=
 =?us-ascii?Q?wlFh/MT05AU4CRBRGpuaWO+V4W9K6eIjmtdjCIWuPdpOgv+YuU6uC+1mXWpV?=
 =?us-ascii?Q?g4/t8LZ5Ovo5hnW49K9hbyngszSjh9uqbc3z7gWptKqq2AcN3a64SZxSX5eU?=
 =?us-ascii?Q?1l1TTYx1mkCamtW07ox3NwvmTO5xvPb1sarmFVXscYjyCu2n1oZlNhp2v73T?=
 =?us-ascii?Q?ASdpgYyoqYkpSYl2s16SKNiMjlMG8X5mgBYJLNvGK864hFJjRJpeHQzPeqti?=
 =?us-ascii?Q?clrzzbN/NKOJgv0LgFQJciuXGpvePwMvvB88Yfjz6xA28s36mxsfq24cZXBv?=
 =?us-ascii?Q?TgO6nAlDwVf7F3v93BrcznMUnaSN7TGxeLOJWscihLtd1Xh54wGv3QpbDpcR?=
 =?us-ascii?Q?4zBS0MLgSGt5hjD7vIKFl8vL63Kwqv+S4SARySfvLVONR85dX2N8jbYEdKcE?=
 =?us-ascii?Q?mEMVVLgXVOUdlx27n63wcYQS+AoacazJ1ntjDLdpx9iA0mcEt3Ts6VLU50Px?=
 =?us-ascii?Q?Xwu+X13HuQuKqltnXp7vQe34S+hAb4oULAWfuRMR2sl4sbYyavBC4XWP4Fmu?=
 =?us-ascii?Q?eX/FgdZYxGD8iXlp51qY8f5oAPxVPfzX0EkbV1DdIcnPwqkW+jamJYBn638K?=
 =?us-ascii?Q?AUn+PsvWcleWI9Mz/Gi7HFjX0JxCMNCB+TwMhvkkMHP+8W0jVNiMTy9w3ge1?=
 =?us-ascii?Q?d/oUTkkKPlWxd4RrpLWhThSfil3Vf17HiPSxBLPw6hPGyxiQJ5Kuoa8ER7nq?=
 =?us-ascii?Q?m3D1husErP/g4q/4RLj/oneww9GQ3txpPSA3XOuMa+MPA4g1rS2s7xA6i8Ix?=
 =?us-ascii?Q?V/WN2z+5gK4mh9UHiM+9itAZ66wJOX/meXI2EywB2oEPm4ZB3qp4dlfLVxV1?=
 =?us-ascii?Q?piRuXqvCCYacpA/XCGN7T0kVraCZGBNizCnRUfbalhuYuFLJwfOIth7vk32C?=
 =?us-ascii?Q?MnT0ra2S84Z09iOEbjiOL+UvB8DjClMHA47d63XkqjcLNol5aZf+zsuqaoYi?=
 =?us-ascii?Q?+9XXhl9Om4PawY8Gc7tVL+wjld4KNAX1BdslVaY3A4B7qsKgfrcGmdhs1XY0?=
 =?us-ascii?Q?b6f1Bet1bPQOipmoQ00wxUbm/VLfXcoGM4ufFx/cCS0BVxBPMumnCsBtIO/O?=
 =?us-ascii?Q?b/MzPoNL47IDZBmd5gQW1ZoRGEybtZBBvhQDei3YtsEJ4G/ughGB8WPHxR4I?=
 =?us-ascii?Q?RV/M91Ww5O+7hsJvtF9V67/RJy9WsIsFbQ5H/YecoiTnHFlTdjnFHjJtyaW+?=
 =?us-ascii?Q?ZvBHFNhfVH0ztjgRUdp/5xl6J4z0k4XxvdEJW9XHuvAtUjTXQ+Dbiea1QCY8?=
 =?us-ascii?Q?5jAytQNpnJyt5vk9mkaN726wCV7NElO0YoUpyws0l5FH9xcu/cPeM+bZe3vL?=
 =?us-ascii?Q?yfiaJdo9N8BDhsw6wtXk5ODhNBASguU=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d94af979-d9c4-421c-e3bb-08da22c4ba7c
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2022 11:55:56.9339
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Kl/NobSDfc8cZc05Eq4/U7VJztciTF/bL8ydmavSkIczSctTggC1LfaiQCqK3zoLSw5XYu7b6nMYhTknsWA9ecSnk9Uy+xrwHDZINr3XmlQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR10MB3668
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-20_03:2022-04-20,2022-04-20 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 adultscore=0
 malwarescore=0 mlxlogscore=711 suspectscore=0 spamscore=0 bulkscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2204200072
X-Proofpoint-GUID: IciD0fSwFGn2CFsujZ-u1g7U1NSg8sQV
X-Proofpoint-ORIG-GUID: IciD0fSwFGn2CFsujZ-u1g7U1NSg8sQV
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

As a kernel community, I don't think we are pro-active enough about
preventing bugs.  One of my co-workers and I have started a bi weekly
phone call to look at CVEs from the previous week and discuss how they
could have been prevented or detected earlier.  Of course, my solutions
are always centered around static analysis because that's my deal but
some bugs are caused by process issues, or could be detected with better
testing.

In this case there were two bugs proposed bugs.

1) The memory leak that Fabio noticed.  Smatch is bad at detecting
   memory leaks.  It's a hard problem, because in this case it's
   across function boundaries.

Fabio caught the leak.  I don't know if I would have.

2) The NULL dereference.

   The "&pnetwork->list" expression is not a dereference so this is also
   a cross function thing.  I thought I used to have an unpublished check
   for bogus addresses like that where pnetwork is NULL.

   Another is idea is that when you have pnetwork++ and it's a NULL
   pointer then print an error message.  Or even potentially NULL.
   There are various heuristics to use which mean that "A reasonable
   person would think this could be NULL".

   Or another idea would be that we could test patches.  Right now we
   don't really have a way to test these.  But, of course, we wish we
   did.

It's not super likely that we would have committed the NULL deref
patch.  I would have caught that one if you didn't and Fabio likely
would have as well.  But I like to remove the human error whenever I
can.

regards,
dan carpenter