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;
Message-ID: <ba1652bb-4bf1-95c0-683e-474d07edad75@oracle.com>
Date:   Tue, 18 Jul 2023 19:08:48 +0530
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.12.0
Subject: Re: [PATCH next] media: i2c: fix error handling in
 ub960_rxport_add_serializer()
Content-Language: en-US
To:     Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     dan.carpenter@linaro.org, kernel-janitors@vger.kernel.org,
        error27@gmail.com
References: <20230718085846.3988564-1-harshit.m.mogalapalli@oracle.com>
 <159ca9b9-0702-b347-3e4f-66272dfce55e@ideasonboard.com>
From:   Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
In-Reply-To: <159ca9b9-0702-b347-3e4f-66272dfce55e@ideasonboard.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U1Uzc1J6dHpYWldFQWtqQW1Kb2txdmUxVExCaGdkTUNoUFBBeHdjcFg4UkR1?=
 =?utf-8?B?UFE1YzgxL0V1bTlvLzd0bm4yQXluMm9QSlp3MTd6U2RNeWwxVWs4TVh6bEk4?=
 =?utf-8?B?Sm91d29zL3lIU2NUQ1BhWHA0d0hRSWhpbXBnNkNLT0tGaHFHSjMxUWVtZXZ4?=
 =?utf-8?B?ekVBVnRncWJ1VEczTFJDZjdzMUN2K3I4YmVLd1FZYTg0ajlidmFNSkNrY1Fz?=
 =?utf-8?B?cUVBSlhNVVJBdWljZHJVZWVrUGJEcG5odHBJaU9WZG52eHV0VUNRNXhpU1RJ?=
 =?utf-8?B?Wk5JVDduUUhKR2ZtT2xWc2RvTlFlbnM4dFFtTWM2bzVaZjJlcHRkUm5pVTNO?=
 =?utf-8?B?MmFZcGQraWVOUW9VNTNDdmhUZnZDa0xHaXZOcmJHWWVzTXNTSEJZTzJ1RmpQ?=
 =?utf-8?B?MmNqL1VOOGlGWi92cTBmNjVuWXBQU1JzZXJDUm9ZUE94YnFwemV3V1dHU0tr?=
 =?utf-8?B?ZGVmM0o2ckFJYWFqUVVRSXArQzNYQ0hGaEkxYzhEWFR0NlVaUFNPMi9JVEti?=
 =?utf-8?B?ZGRxU3l0QU5lYjJmYThFbEZobUJ4TmlIaFNiN2F2VGxqMzBtWmpPTTlFKy85?=
 =?utf-8?B?TzVQQWF3WU5iSG5MWmpidDQwc0dzTDZaM292dGtsbTBPcllQdDNleTB3T005?=
 =?utf-8?B?TmFiSXhEK3lqb3drV1VDZVNwWllsM1FRR0ZXN0d4QjNwUUtSZUlLbC93V1Fo?=
 =?utf-8?B?V2s0aU5UbngwK01RNUttb3BBemhVR1RaYTZVeUpLeUh6REd2OHJuTHBJKyts?=
 =?utf-8?B?N2ZGZmVIM2xsNHM2WHUzcjZJazY4OXFhMUw5bXdRMlM1MVU4ejhsNEh2aDAv?=
 =?utf-8?B?QVBMc05NYVAwa3hnVndHSldya01KVmx1MTVJV1Z4a21TSFc3SkVvMzlOWCs1?=
 =?utf-8?B?SHBkMllYcFBxRko1UG9VNnBpdm83TzZ4QklXdjRDV2VhODBldnBLVkRVRmVO?=
 =?utf-8?B?SlA0NUNYYkhqUFJhTlZCckoxT3dNeTE4L3JDSEhscmtWd054SjFoVkN1UUlh?=
 =?utf-8?B?YmRoZTk2Q3dDOEw2NVp4Q2hWOEpRYkRnbDBpbU5jT09OR3FoUlZBV0lYa2ZZ?=
 =?utf-8?B?bmQ1MURFbXg3dzkyb2xUamJ6Qk1JUWliZ1NHOVlPWDNRSFB4ZmdNZVBNK2ly?=
 =?utf-8?B?cnlydnFWcC9vWGxkZWRqOTdOWTJLU1hTeU0yb3RQTGRjQkFoRmhJMWpjT09U?=
 =?utf-8?B?OGxoR0F3eldVQUgzdDE5WUo1RTFqUlgxMDZGbzNOV0t0dkp5RzJ0Uk4yNzZJ?=
 =?utf-8?B?UXZjRkRSczRMUWs2Wit1STh4ZHhLTUJMZHUzNGlCSjZuTjZKSllmK01sWU81?=
 =?utf-8?B?SXRwMG5YUEdlWnRodjdoKzI4NEc2Mzk3TFpoZ0U5aWhvdTlLdXl1TzAwZERu?=
 =?utf-8?B?VnE0UHlsOXROR0VFMkNMeVBEQmpYa0NHYVV1MHpHVUp5aWJjci9yY3I5bkVB?=
 =?utf-8?B?aUNvaTUxQXZ5T1FmSW95TTRDOXcvQ0F6SmI1WmxCbytyM3ArYVhuc2xYU0VF?=
 =?utf-8?B?Nkk0RWlEN2RwTVVNdEloOWl0NlJUV0orK1J3TEZZMXl1UGR6R2JOOU1RcmZ4?=
 =?utf-8?B?SkRmeFJmVFlmejNVNUVaK3hYMHZXYTU0dys0cXEvdGtLT0pRaFNmdEE2SUZF?=
 =?utf-8?B?UmlRNlFyNmhKa0tFZWJBdVgrTGhTT3dGSy9SUlh2Zkc1N3c3UnMxUmlzTE5l?=
 =?utf-8?B?SXloRTN2YnNhMVdrNWxiWDBKczFFU3k4Y1B2TjcvaS9tMU9VeDRmV04xdTBk?=
 =?utf-8?B?OXEzZm9pVEthenRvOEhCNVlpMURkdk5pU0NIMDU1Rk5GRFFjd29jb3JaOUVs?=
 =?utf-8?B?Z3lSVlVkRmQzNldTY3BNUk4wZ0QzSDFvYnp2M0xkWGhPQnVUYTRaZ2lzVlpB?=
 =?utf-8?B?M2xVS0Y5VTR4ek5MSmpYTkdGUnlSekx0eVZFNE1tMmJxUVo0b1dCUkgrekp0?=
 =?utf-8?B?eHJ5cG41UFAxWTNUcDBjM0c5UnA3VWxNR0ZjL2x5VGpEWHovZnlMMURVTHdJ?=
 =?utf-8?B?b0djRW8rTmQ4Q25FSVIzSkRjcnRFa2x5dVFWd2l1SllGTHJ5ODk1RzNoc09D?=
 =?utf-8?B?Z1lWa0xxYWtOU3BDWnR4cStMS3BSZGZTVnp3dVFFRTBIcEQrd2dMUE9qSGov?=
 =?utf-8?B?Zk9xQ1NscHBiaDRmWEYwTlBPQ2FmMTI2ei9FVjBIcTVuTDAydVlTNWl2dUdq?=
 =?utf-8?Q?kqxyrOFvaGsRDRqK8QI2cVY=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?dFFNQzhCaWlBVURIVHpPcXhwdEI4enhicXlCb1M2emZDbzBZaVVGRnVGTFdX?=
 =?utf-8?B?ZnZ6VW1vclRDSkt4RXpQZ2NrMlJJNzBuTVF0MTRDRDNma2x1aStLcVBURjc0?=
 =?utf-8?B?aW9ya084S29ZajJOU2x2VXhQK2tOL0hIcC9BOGdJMEdSdWZpWlFHZm9iYkRj?=
 =?utf-8?B?WkpqUC9kbzkrMjdCalVOQzVGUk5qTjVUOVVORTRzVENKbkpiNHJJSE02VU5p?=
 =?utf-8?B?RWRTaFBJMzh6aEd5eTVSalU0ZHl4Z0o4N3p3VDlYMDVmOTVIZmFiS1ZzalY0?=
 =?utf-8?B?S2RrK0hsb0VoNGVmaXZoN1ZHZHJaSFlXa0RGSUxDSlllVnRjRytaUnhMSWM4?=
 =?utf-8?B?Z093SXEwZzJGT2lJaTRMRnZQRXVINjAxb3AvbFNkNzBBNGZoRXlzdWFOU1ZB?=
 =?utf-8?B?OG1xNE04N3pKVWVFY1cycU1BcC92YmFkYVM1R0ZZNER1WU40R1dzVEMxTmZt?=
 =?utf-8?B?amszdjNWbm1XSmtJTkM5bUFXa2RvcTI5aFRYRUpSWjg4V2NJQkFNbEZVcXNi?=
 =?utf-8?B?Ym1zbGkzNWFRRjd2T2EzN0l0MlJHVEVPcElrN29ZSFlBUUppZnk1WndNWFl2?=
 =?utf-8?B?NElFbzJqL2x1cTh2SlRnWWY5K0EyZTZvK3UraEdXSTgrODRBcE80UHBhWitK?=
 =?utf-8?B?TlMxNUcrSGE3SDJuZTNxYTJZeXJSOS9NdGpFNzIwQTRudnA4N3M4NnhtNXJN?=
 =?utf-8?B?WDVtUWt4UW9qWVp4eldzOG5RVmhITG1JVTNUM2hSWHNrbjBpc2pmWW5iZFpT?=
 =?utf-8?B?a1FTb0pJQ04rdFlXRlJzNGcwM3FKTjkyMHBKdHAvYXVvcmhiY0VWKzRJTC9U?=
 =?utf-8?B?N2dJS3JxL1JjUzdXMERNYXdmczVmbjdIVzgyQkg5bGRRZUpHUWVxTEoxZnNX?=
 =?utf-8?B?Y0g5M3p1b294cFp3VHFEMlQyK2dPZlFqeDdRc3ZkeTZVdEVDU2szNjBJVmY0?=
 =?utf-8?B?d3VZemU2eCs3a0FlWTVXN3VXMlNiUHdmK21iM0N0TjRieEtjUHVGTndwc290?=
 =?utf-8?B?Y0RhYkpGV3h3WGhsOFkrMll4eHQxTFF5NmZ5SDhmZ2hIQmFZUEZFZUMxQzAz?=
 =?utf-8?B?Q0Ixc2dwbmgyUlJ2RnV6VWp6bStCT21pMGRqZ1IrMTVsVnZxakNLdWpicnUv?=
 =?utf-8?B?WVRyUDhkazVBT1JJV3ZWQUZLbkxhTWx4TnRqRVJYRjFlOVNhdExxVWRPTDBN?=
 =?utf-8?B?VWtUWVlkQ0o5UFZjaFFtRnloeVFUY3ZQeWdMZUJXUWV6VFJaVVVQVkM1YzE2?=
 =?utf-8?Q?NQKEUjiA6AYpeGo?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: efc5a723-90f4-420a-9e15-08db8794579a
X-MS-Exchange-CrossTenant-AuthSource: PH8PR10MB6290.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jul 2023 13:39:00.0457
 (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: TY3MrxG14NcpNFTA6+E/ZX929aZTGjYhKkO+YU8ASXlwObQieXLbH2kV+/8F8IkZX4tut29SDxxneBNA9+wKKkh229JpOqxYHsyZZciWqR9PBS91ASh2B4yLZ3B/FRZa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR10MB5037
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-18_10,2023-07-18_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 adultscore=0 mlxscore=0
 bulkscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2306200000
 definitions=main-2307180125
X-Proofpoint-GUID: 2eC6ZAeO3BQbBh83EGhZ83j4By26YBmK
X-Proofpoint-ORIG-GUID: 2eC6ZAeO3BQbBh83EGhZ83j4By26YBmK
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE 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

Hi Tomi,

On 18/07/23 7:01 pm, Tomi Valkeinen wrote:
> Hi,
> 
> On 18/07/2023 11:58, Harshit Mogalapalli wrote:
>> Smatch warns:
>>   drivers/media/i2c/ds90ub960.c:1671 ub960_rxport_add_serializer():
>>   err: 'rxport->ser.client' dereferencing possible ERR_PTR()
>>
>> i2c_new_client_device() returns error pointers on failure and in
>> dev_dbg statement we are dereferencing error pointer which is a bug.
>>
>> Fix this by using IS_ERR() which checks for error pointers.
>>
>> Fixes: afe267f2d368 ("media: i2c: add DS90UB960 driver")
>> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
>> ---
>> Found with static analysis, only compile tested. Although in
>> drivers/media i2c_client_has_driver() checks are present, IS_ERR() would
>> probably be sufficient here.
>> ---
>>   drivers/media/i2c/ds90ub960.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/ds90ub960.c 
>> b/drivers/media/i2c/ds90ub960.c
>> index e101bcf2356a..88144e3ec183 100644
>> --- a/drivers/media/i2c/ds90ub960.c
>> +++ b/drivers/media/i2c/ds90ub960.c
>> @@ -1662,7 +1662,7 @@ static int ub960_rxport_add_serializer(struct 
>> ub960_data *priv, u8 nport)
>>       ser_info.addr = rxport->ser.alias;
>>       rxport->ser.client =
>>           i2c_new_client_device(priv->client->adapter, &ser_info);
>> -    if (!rxport->ser.client) {
>> +    if (IS_ERR(rxport->ser.client)) {
>>           dev_err(dev, "rx%u: cannot add %s i2c device", nport,
>>               ser_info.type);
>>           return -EIO;
> 
> Good catch. I think this should be modified to return 
> PTR_ERR(rxport->ser.client) instead of -EIO. Do you want to update the 
> patch or shall I do the change?

Thanks,

I think it is easier for me to do that change PTR_ERR change. Will send 
V2 with that change added.

Regards,
Harshit

> 
>   Tomi
>