| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f1c717cf-63eb-402a-82cc-91c445055b97@stanley.mountain>
Date: Fri, 14 Feb 2025 17:42:25 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: oe-kbuild@...ts.linux.dev, Karan Tilak Kumar <kartilak@...co.com>
Cc: lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
linux-kernel@...r.kernel.org,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Sesidhar Baddela <sebaddel@...co.com>,
Gian Carlo Boffa <gcboffa@...co.com>,
Arulprabhu Ponnusamy <arulponn@...co.com>,
Arun Easi <aeasi@...co.com>
Subject: drivers/scsi/fnic/fdls_disc.c:263
fdls_schedule_oxid_free_retry_work() warn: inconsistent indenting
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 128c8f96eb8638c060cd3532dc394d046ce64fe1
commit: a63e78eb2b0f654b138abfc323f6bd7573e26145 scsi: fnic: Add support for fabric based solicited requests and responses
config: i386-randconfig-141-20250214 (https://download.01.org/0day-ci/archive/20250214/202502141403.1PcpwyJp-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
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@...el.com>
| Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
| Closes: https://lore.kernel.org/r/202502141403.1PcpwyJp-lkp@intel.com/
smatch warnings:
drivers/scsi/fnic/fdls_disc.c:263 fdls_schedule_oxid_free_retry_work() warn: inconsistent indenting
drivers/scsi/fnic/fdls_disc.c:989 fdls_send_fabric_logo() warn: inconsistent indenting
drivers/scsi/fnic/fdls_disc.c:1953 fnic_fdls_validate_and_get_frame_type() warn: inconsistent indenting
vim +/cur_jiffies +166 drivers/scsi/fnic/fdls_disc.c
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 244 void fdls_schedule_oxid_free_retry_work(struct work_struct *work)
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 245 {
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 246 struct fnic_oxid_pool_s *oxid_pool = container_of(work,
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 247 struct fnic_oxid_pool_s, schedule_oxid_free_retry.work);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 248 struct fnic_iport_s *iport = container_of(oxid_pool,
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 249 struct fnic_iport_s, oxid_pool);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 250 struct fnic *fnic = iport->fnic;
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 251 struct reclaim_entry_s *reclaim_entry;
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 252 unsigned long delay_j = msecs_to_jiffies(OXID_RECLAIM_TOV(iport));
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 253 int idx;
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 254
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 255 spin_lock_irqsave(&fnic->fnic_lock, fnic->lock_flags);
Passing fnic->lock_flags is wrong. spin_lock_irqsave() is not nestable
in the sense that the "flags" variable can't hold two values at once:
orig_irq = save_original irq states and disable()
orig_irq = save_original irq states and disable()
restore(orig_irq)
restore(orig_irq)
The second restore(orig_irq) is not going to restore the original states
it's going to leave them as disabled.
If fnic->lock_flags is holding anything useful when this is called, then it
is a bug or if anything uses it before we exit that will break us. Just
declare "unsigned long flags;" locally.
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 256
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 257 for_each_set_bit(idx, oxid_pool->pending_schedule_free, FNIC_OXID_POOL_SZ) {
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 258
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 259 FNIC_FCS_DBG(KERN_INFO, fnic->lport->host, fnic->fnic_num,
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 260 "Schedule oxid free. oxid idx: %d\n", idx);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 261
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 262 spin_unlock_irqrestore(&fnic->fnic_lock, fnic->lock_flags);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 @263 reclaim_entry = (struct reclaim_entry_s *)
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 264 kzalloc(sizeof(struct reclaim_entry_s), GFP_KERNEL);
The indenting is off. The normal way to write this is:
reclaim_entry = kzalloc(sizeof(*reclaim_entry), GFP_KERNEL);
Remove the cast and change the sizeof().
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 265 spin_lock_irqsave(&fnic->fnic_lock, fnic->lock_flags);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 266
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 267 if (!reclaim_entry) {
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 268 FNIC_FCS_DBG(KERN_WARNING, fnic->lport->host, fnic->fnic_num,
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 269 "Failed to allocate memory for reclaim struct for oxid idx: 0x%x\n",
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 270 idx);
Remove this DBG. The truth is that for as long as anyone can remember
the kmalloc() allocator won't fail for tiny allocations. We're more
likely to formalize this rule than we are to change it at this point.
Still, we check for errors until that day arrives.
But allocations are not supposed to print an error message on error.
It's just messy and it doesn't add any information that isn't already
in the stack traces that kmalloc() prints.
I guess we can't call schedule_delayed_work() without holding the
spin_lock? It's a strange thing.
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 271
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 272 schedule_delayed_work(&oxid_pool->schedule_oxid_free_retry,
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 273 msecs_to_jiffies(SCHEDULE_OXID_FREE_RETRY_TIME));
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 274 spin_unlock_irqrestore(&fnic->fnic_lock, fnic->lock_flags);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 275 return;
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 276 }
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 277
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 278 if (test_and_clear_bit(idx, oxid_pool->pending_schedule_free)) {
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 279 reclaim_entry->oxid_idx = idx;
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 280 reclaim_entry->expires = round_jiffies(jiffies + delay_j);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 281 list_add_tail(&reclaim_entry->links, &oxid_pool->oxid_reclaim_list);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 282 schedule_delayed_work(&oxid_pool->oxid_reclaim_work, delay_j);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 283 } else {
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 284 /* unlikely scenario, free the allocated memory and continue */
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 285 kfree(reclaim_entry);
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 286 }
a63e78eb2b0f65 Karan Tilak Kumar 2024-12-11 287 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Powered by blists - more mailing lists