Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <ae7a82c2-5b19-493a-8d61-cdccb00cf46c@oracle.com>
Date:   Mon, 11 Oct 2021 10:29:46 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
From:   Mike Christie <michael.christie@oracle.com>
Subject: Re: Antw: [EXT] Re: [PATCH] scsi scsi_transport_iscsi.c: fix misuse
 of %llu in scsi_transport_iscsi.c
To:     Ulrich Windl <Ulrich.Windl@rz.uni-regensburg.de>,
        jejb@linux.ibm.com, martin.petersen@oracle.com,
        Chris Leech <cleech@redhat.com>, qtxuning1999@sjtu.edu.cn,
        Lee Duncan <lduncan@suse.com>
Cc:     open-iscsi <open-iscsi@googlegroups.com>,
        linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
References: <20211009030254.205714-1-qtxuning1999@sjtu.edu.cn>
 <5daf69b365e23ceecee911c4d0f2f66a0b9ec95c.camel@perches.com>
 <6163DB2E020000A1000445F1@gwsmtp.uni-regensburg.de>
Content-Language: en-US
In-Reply-To: <6163DB2E020000A1000445F1@gwsmtp.uni-regensburg.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a1ROVkpQMDNvRGlhNEQ3SWp6R082M0x4R3ZDRmF5RlFMcEpJL0l4bnlMVnBn?=
 =?utf-8?B?OVRXY2NQQlZWREZqeEYvYjA3Y0lTd3FwUUF4TmlyNlJQb1RVMkRWVjJGVTJW?=
 =?utf-8?B?enNrdGxjRC9BcU5MTSs2WUZBMzd5SThvZ2c4VHNRSVZMcisxMnIyWXZGeHpT?=
 =?utf-8?B?TG9PZHI0ck5ZRWhQNjltcFc4eHpFYkNWK3pWOW52YjQ5LysxQVNHUUR1VEtH?=
 =?utf-8?B?NXA0QjJ2RnVBN21XeUdnTUFhY1NzQ0h6eU1JWXFoMmtzOGcyYis3aFN3TkNT?=
 =?utf-8?B?RUg4WVJlL1B1MzJuVGVhdDhudCsxS1lwUjNJb21sdU9Wck91b3pxR0d5ZHRt?=
 =?utf-8?B?UFNGVEFYbzl4NlVtQ2xWTjRzY3dDTFRHRVNlSS83K0taTnI3OXlQNXpzOExO?=
 =?utf-8?B?TnF0cXJZUEpNQkk4cjBDZGJFeU5vRnFaUy84eEFxN2V6em5IS04vQU9XTjQ4?=
 =?utf-8?B?YW5OZktZdUNXb2VMQmVhYVZzbUFuMDZ6VEphZkhBZTM3RUJhSnErZ1ZjbzNs?=
 =?utf-8?B?VGw4eSt1S3IrT2VjWWtSZy84TGpEdGZvWUZKVFJXR0o4bTNzb2Z5Q1IzNjdm?=
 =?utf-8?B?T2pPazR1NnFOOWVJWkZCNkVRWllQVjNmdER5cFhjanlqTzd2WUlIZHF2bzBM?=
 =?utf-8?B?WDlUc1N2ald2UzUrbmV1Z01kNUlXb003MFBLTUdvQWhpWjlzemtFRHQ5MjZ3?=
 =?utf-8?B?bFFPWVM1QjBDc01OSnQ4N1dCZ2k5Q2ZRNHBVbTRTTzFzV3lQYU5MSkRBZy9V?=
 =?utf-8?B?VHJ4MlQ1aXRmeVVTSi9wY0lFUzlaR1ljTTgxSGlVcGl4SmJIcEw2RmVCZU9n?=
 =?utf-8?B?YVgrekk3QmNvVU93SFBzT2hzSmNUL3Zxc01WenZISkU0cko4QXVIcWpOUnVF?=
 =?utf-8?B?eS9GQTlDMDl2QUJWOEtzVU9VakxPRW1YMFZScFcvSnc3NVNwTldXSnIzZ3Bi?=
 =?utf-8?B?bXZ5V3dxdThjWWorbnlsakJXdW9SWC80ODJvVmQ3Uk5abmV3a3A5RWlqa2hR?=
 =?utf-8?B?aTlxTzJLdFN1Ry90T3pVWURjeU9lOVVjY014RFhZSjdsZ2QzbzBHK0E5NUE4?=
 =?utf-8?B?eFZBQlVGSjViQU1iTGpWaXBzR3dxR2ltNmJab0RuT0kwaVAwcGx5QkFxbjFs?=
 =?utf-8?B?bnpjM2tkTmVHMmxQMkd1cVlPYkh4UXNiQTRiTzI3V3g5UTBRQ0hGQyt5SmtR?=
 =?utf-8?B?SnBYNkNhenZkR2kvRWNjbDArM3FpcmswWVBMY25ZQnBsVnNBNEtDY29oUFpS?=
 =?utf-8?B?bGVIdlZLTjdkWTJQYWdJMWY5clN6Y2Y3YTY3bnFzcVAzUmJPdmJZTGtmcmJn?=
 =?utf-8?B?WDhCMVJ0Vk56L2tUK1FSM3NteVlyclRuWDNGcEREU0tBQkxGU1dKZTVnTEg5?=
 =?utf-8?B?ZXROeVFVM3JXRVhUVkZ4VmxKRWR3R01rcldvNWoxRFBDSzdPczgranZLVHM5?=
 =?utf-8?B?UTRGenBYOThRY0NTWGdDL0FjTi9BZjZsZ2VXSnFEdU9DZUpJNHhxVWErMmtB?=
 =?utf-8?B?TGNVdFlUK0xUY2tEVFB5bWxoUm5kSUR1bHM1OFRKanlSZGVLT2tYSnFJV0Ez?=
 =?utf-8?B?dU1aWjBEWlg5UDJlK3o0Ym1LSGhKKytOclU4NTNwd2RDTmRsa0FRRWk3RGpJ?=
 =?utf-8?B?azE4QWVwcWEyY3F5aDVJdVZILzFCbmFVeXI0VlhabU4rZUhiTHdKbmgvYTlS?=
 =?utf-8?B?dUk5NXFyWGVwTjl2ZkFlOE5aS0s5NGJOaW1oWjRRclprbm1oUlNYTjY1WTdG?=
 =?utf-8?Q?eB75Hxx/c7qWbS1SQSzTxqGQuoBVDIG8rFOgIvd?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 213403dd-e323-485b-ec20-08d98ccbf629
X-MS-Exchange-CrossTenant-AuthSource: DM5PR10MB1466.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Oct 2021 15:29:48.7237
 (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: G898/D2e2eBWBRCwoRAGfHawO7/nsTCIL8qFIO3ll10FjiV1RnnCJnd7TCXikRWdbHg3kaIp4AuJu7xikiL1TjfsvvTskm3fIbmhOFqEzBM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR10MB3803
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10134 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 suspectscore=0
 mlxlogscore=999 mlxscore=0 bulkscore=0 adultscore=0 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001
 definitions=main-2110110090
X-Proofpoint-GUID: JwKZ38PnUqpCQmfQci3CxYYb2jtA-FJa
X-Proofpoint-ORIG-GUID: JwKZ38PnUqpCQmfQci3CxYYb2jtA-FJa
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/11/21 1:35 AM, Ulrich Windl wrote:
>>>> Joe Perches <joe@perches.com> schrieb am 09.10.2021 um 05:14 in Nachricht
> <5daf69b365e23ceecee911c4d0f2f66a0b9ec95c.camel@perches.com>:
>> On Sat, 2021-10-09 at 11:02 +0800, Guo Zhi wrote:
>>> Pointers should be printed with %p or %px rather than
>>> cast to (unsigned long long) and printed with %llu.
>>> Change %llu to %p to print the pointer into sysfs.
>> ][]
>>> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
>> b/drivers/scsi/scsi_transport_iscsi.c
>> []
>>> @@ -129,8 +129,8 @@ show_transport_handle(struct device *dev, struct 
>> device_attribute *attr,
>>>  
>>>
>>>  	if (!capable(CAP_SYS_ADMIN))
>>>  		return -EACCES;
>>> -	return sysfs_emit(buf, "%llu\n",
>>> -		  (unsigned long long)iscsi_handle(priv->iscsi_transport));
>>> +	return sysfs_emit(buf, "%p\n",
>>> +		iscsi_ptr(priv->iscsi_transport));
>>
>> iscsi_transport is a pointer isn't it?
>>
>> so why not just
>>
>> 	return sysfs_emit(buf, "%p\n", priv->iscsi_transport);
> 
> Isn't the difference that %p outputs hex, while %u outputs decimal?
> 

Yeah, I think this patch will break userspace, because it doesn't know it's
a pointer. It could be doing:

sscanf(str, "%llu", &val);

The value is just later passed back to the kernel to look up a driver in
iscsi_if_transport_lookup:

        list_for_each_entry(priv, &iscsi_transports, list) {
                if (tt == priv->iscsi_transport) {

so we could just replace priv->transport with an int and use an ida to assign
the value.