Re: [PATCH v2 11/13] ata: libata-core: Reuse available ata_port print_ids

[Niklas Cassel <cassel@kernel.org>]

On Sat, Jun 29, 2024 at 12:31:57AM +0800, kernel test robot wrote:
> Hi Niklas,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on linus/master]
> [also build test WARNING on v6.10-rc5 next-20240627]
> [cannot apply to mkp-scsi/for-next jejb-scsi/for-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Niklas-Cassel/ata-libata-core-Fix-null-pointer-dereference-on-error/20240627-123023
> base:   linus/master
> patch link:    https://lore.kernel.org/r/20240626180031.4050226-26-cassel%40kernel.org
> patch subject: [PATCH v2 11/13] ata: libata-core: Reuse available ata_port print_ids
> config: i386-randconfig-141-20240628 (https://download.01.org/0day-ci/archive/20240629/202406290027.cdgsPQAF-lkp@intel.com/config)
> compiler: gcc-8 (Ubuntu 8.4.0-3ubuntu2) 8.4.0
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202406290027.cdgsPQAF-lkp@intel.com/
> 
> smatch warnings:
> drivers/ata/libata-core.c:5467 ata_port_alloc() warn: unsigned 'ap->print_id' is never less than zero.
> 
> vim +5467 drivers/ata/libata-core.c
> 
>   5443	
>   5444	/**
>   5445	 *	ata_port_alloc - allocate and initialize basic ATA port resources
>   5446	 *	@host: ATA host this allocated port belongs to
>   5447	 *
>   5448	 *	Allocate and initialize basic ATA port resources.
>   5449	 *
>   5450	 *	RETURNS:
>   5451	 *	Allocate ATA port on success, NULL on failure.
>   5452	 *
>   5453	 *	LOCKING:
>   5454	 *	Inherited from calling layer (may sleep).
>   5455	 */
>   5456	struct ata_port *ata_port_alloc(struct ata_host *host)
>   5457	{
>   5458		struct ata_port *ap;
>   5459	
>   5460		ap = kzalloc(sizeof(*ap), GFP_KERNEL);
>   5461		if (!ap)
>   5462			return NULL;
>   5463	
>   5464		ap->pflags |= ATA_PFLAG_INITIALIZING | ATA_PFLAG_FROZEN;
>   5465		ap->lock = &host->lock;
>   5466		ap->print_id = ida_alloc_min(&ata_ida, 1, GFP_KERNEL);
> > 5467		if (ap->print_id < 0) {

Well, the check is correct, but since ap->print_id is unsigned int,
we will need to use a temporary (signed) variable to check for errors
from ida_alloc_min(). Will fix in next revision.


>   5468			kfree(ap);
>   5469			return NULL;
>   5470		}
>   5471		ap->host = host;
>   5472		ap->dev = host->dev;
>   5473	
>   5474		mutex_init(&ap->scsi_scan_mutex);
>   5475		INIT_DELAYED_WORK(&ap->hotplug_task, ata_scsi_hotplug);
>   5476		INIT_DELAYED_WORK(&ap->scsi_rescan_task, ata_scsi_dev_rescan);
>   5477		INIT_LIST_HEAD(&ap->eh_done_q);
>   5478		init_waitqueue_head(&ap->eh_wait_q);
>   5479		init_completion(&ap->park_req_pending);
>   5480		timer_setup(&ap->fastdrain_timer, ata_eh_fastdrain_timerfn,
>   5481			    TIMER_DEFERRABLE);
>   5482	
>   5483		ap->cbl = ATA_CBL_NONE;
>   5484	
>   5485		ata_link_init(ap, &ap->link, 0);
>   5486	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki