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 20:39:26 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Jaehee Park <jhpark1013@gmail.com>
Cc:     Pavel Skripkin <paskripkin@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: <20220420173926.GE2969@kadam>
References: <cover.1650392020.git.jhpark1013@gmail.com>
 <3c5a510938ac395a13bb09c0de1868cce8ca3dd8.1650392020.git.jhpark1013@gmail.com>
 <32587233-0ff6-ed0f-b873-cd4f797005a9@gmail.com>
 <20220420115534.GF2951@kadam>
 <20220420144834.GA1313590@jaehee-ThinkPad-X1-Extreme>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220420144834.GA1313590@jaehee-ThinkPad-X1-Extreme>
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?OBVyqWiGolo7yl+sEBWUJzCJ20Va+VOimF/QBqSkpcHgk4c5uJYo3gmqfLPh?=
 =?us-ascii?Q?X7cBDlr6SzqVqX+aozbSOfFktgA16G6J3jO54yxXgf/Ww/YYnYvZ+AQ+TlJE?=
 =?us-ascii?Q?jQ48Ga3sMIdduzYGW+kVnMN5/LN1cFeWH0DGBdWy7a4JOQlsOPchGd0tkZ9m?=
 =?us-ascii?Q?OlduXCGvPaZrZbmQZmlZ6veXep7hHyQI5VY0JnYn3QOBdGmvmIbs7LqBC6VO?=
 =?us-ascii?Q?Jlh25XUZTtGOmeO89sFmBI7t5sqTxyJBk1rMJmJ7snrq4o5SCdQ8YoRHgOiA?=
 =?us-ascii?Q?A/qY+EEpyWbHNZZC0vMSjGqlEZsMX/1qhabwW6eVE7VDPLVsOnL2f9PC6/Uo?=
 =?us-ascii?Q?4JiIbHNWzoz9nBoa7mbPjIbozh5uOUCFaQabHWqnrINd8EDx0C6eYBdDCvnF?=
 =?us-ascii?Q?UKHyEsOw17M/C+McJRO7fsDfeqvOkD9mFFiIFQ9bO9AvdFeXpBis9vwigzkS?=
 =?us-ascii?Q?O8oJ36hFZjsfa0GqPEIqelbmmKqOWi8LbtTneulZVoZpfkcrrJH+UnJyUIuZ?=
 =?us-ascii?Q?yR8MgJ+HnSenvj4BGZKCbAeG9Tm5CNvNtUGxvyWMjhg3vmzTLoklJziSZD2A?=
 =?us-ascii?Q?vjLNADB5P2f6TKLxqubtmTtv72vLnK5e/WRWBNbbjcMQ+b1AK2035biCM/JV?=
 =?us-ascii?Q?G2qVLABb1KwTJChuknU8UZjSCbHEseyRy2Gv6QmnQRbVUKinugX8dmLkn9GL?=
 =?us-ascii?Q?cRFwKCfRxFhK+jRIKCkstznk735YSlf+wnhUaVgchVvr1U30Wdfq54MHyf5S?=
 =?us-ascii?Q?c0TxeTJie4zz7FlF9sa/8IljgtiVyZBmaXkgyJCIGN/56eLHwSeJolrnKcXw?=
 =?us-ascii?Q?WLW+MiZkHxGdNDmQuje0qnFwlgTzP9Yv3kBAJrlLFjJb/i5AddL5bpI7U4ql?=
 =?us-ascii?Q?fZCflPE4UnRJ5gxx493C8A+bsMAfETkxSlPVqKIRrSKUioejFWBjIy/oCkyq?=
 =?us-ascii?Q?C/gzMW4CmSCUbNjjHIfEydd4ad+QXEGudLKf/aQYOY4n0SCO9leDt7Db9U1g?=
 =?us-ascii?Q?pzcYsRyY+RWTnKsDthPC3MForqUs+mLTfqDIE50gbU5F3xMU7B88tt8nha6j?=
 =?us-ascii?Q?/qn0x3aCemI/zzPXeUlH3DoSg8lW00+iOidIJnD2R7PJEAmLxcP66RFkVSwO?=
 =?us-ascii?Q?ppnmSCDQ0VyDNv38ANVq8QjFsGfilw/Z5+VObamfULOH3QZB/8EHZsPMde+r?=
 =?us-ascii?Q?q9NU9+cxomAJxJkuskGzKRwXZGYAvZabIQHXxZkNe4f7wNjqEeiu0J58SFWj?=
 =?us-ascii?Q?bv5EvBDJvFLD/FnqN2MeWg+fEXBSrw1jGymuxAGLwKI2xyhoBXpW1hExpx1c?=
 =?us-ascii?Q?BjIDouy4+Tg7sWVAzCGiyZVxgV+kbp5GD0F23C1hi9oLKssiHIbawMewKbsv?=
 =?us-ascii?Q?WJ+lcb5yPONDWVJQfFwk9yggLrav+E6QgDa0jnESZhuJomLtoKBKiCGJJf4J?=
 =?us-ascii?Q?HA3iJnvuiBGsZ45FzLlXuDJM8VLXV2NpDCjxD/iZfqteAD1mT2pYxulyQfnZ?=
 =?us-ascii?Q?n6eRucg79TdeChRgkfn7VLuPpUoeLBv3NCcHTwY2MreSC7ziw/xvN62VgTt+?=
 =?us-ascii?Q?oyCKWHuVwMYhwaO/TFs7EJDnvSAbelWyOjrGr8lGu5vUjBzatKGDpo9EGCdE?=
 =?us-ascii?Q?UFqU80d8u2kRUCVYU/yUlPBAbEQM3qQF+CZOvbP+i31k9DfbSaQ+65/BmjHB?=
 =?us-ascii?Q?zNipEUqya32tJAnOmZRKg4VVUA9tnf6KR6ijOHwnB2TOdZgHWnc+vxpJDkJJ?=
 =?us-ascii?Q?N2/4O660c9g4Guw1x5j0MsXyCmmcL3o=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 54993d95-44dd-4871-e407-08da22f4c34b
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Apr 2022 17:39:47.1960
 (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: 9Odh6B8lznjBjvtWwGvQDZDJJHWtwFToBlppCQstRrCRDaLh2fj9k2mjV81NJi4d5wzGDjCtYWVHcDhAbDc3LNprxdWLcRYptlWbQx4L9qY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1001MB2196
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-20_05:2022-04-20,2022-04-20 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 suspectscore=0
 malwarescore=0 mlxlogscore=969 adultscore=0 mlxscore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204200104
X-Proofpoint-GUID: ySf6j5fFn0DNupm9qPm3_9fyINimMFul
X-Proofpoint-ORIG-GUID: ySf6j5fFn0DNupm9qPm3_9fyINimMFul
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

On Wed, Apr 20, 2022 at 10:48:34AM -0400, Jaehee Park wrote:
> On Wed, Apr 20, 2022 at 02:55:34PM +0300, Dan Carpenter wrote:
> > 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
> 
> I'm sorry about the NULL dereference. I wasn't sure about pnetwork
> and I should've asked. I wanted to ask why pbuf should be removed
> when it was being used for pnetwork but ended up not asking and
> sent a faulty patch. Sorry again and thank you for explaining the
> errors. I will be more careful about memory leaks and dereferencing
> errors. Are there checks that I can run to detect these?

I was going to suggest that you could write a check yourself, but then
when I looked at it it became quite complicated.  :P

How about I write the check and send you the output tomorrow?

regards,
dan carpenter