[bug report] afs: Fold the afs_addr_cursor struct in

[Dan Carpenter]

Hello David Howells,

The patch 7bd4353ec32a: "afs: Fold the afs_addr_cursor struct in"
from Oct 20, 2023 (linux-next), leads to the following Smatch static
checker warning:

fs/afs/rotate.c:664 afs_select_fileserver() warn: assigning signed to unsigned: '*&(alist->preferred) = (op->addr_index)' '(-1)'
fs/afs/rotate.c:698 afs_select_fileserver() warn: assigning signed to unsigned: '*&(alist->preferred) = (op->addr_index)' '(-1)'

fs/afs/rotate.c
    582                 vnode->cb_server = server;
    583                 vnode->cb_v_check = atomic_read(&vnode->volume->cb_v_break);
    584                 vnode->cb_expires_at = AFS_NO_CB_PROMISE;
    585         }
    586 
    587 retry_server:
    588         op->addr_tried = 0;
    589         op->addr_index = -1;

There is a kind of type mismatch where addr_index is -1.

    590 
    591 iterate_address:
    592         /* Iterate over the current server's address list to try and find an
    593          * address on which it will respond to us.
    594          */
    595         op->estate = op->server_states[op->server_index].endpoint_state;
    596         set = READ_ONCE(op->estate->responsive_set);
    597         failed = READ_ONCE(op->estate->failed_set);
    598         _debug("iterate ES=%x rs=%lx fs=%lx", op->estate->probe_seq, set, failed);
    599         set &= ~(failed | op->addr_tried);
    600         trace_afs_rotate(op, afs_rotate_trace_iterate_addr, set);
    601         if (!set)
    602                 goto wait_for_more_probe_results;
    603 
    604         alist = op->estate->addresses;
    605         for (i = 0; i < alist->nr_addrs; i++) {
    606                 if (alist->addrs[i].prio > best_prio) {
    607                         addr_index = i;
    608                         best_prio = alist->addrs[i].prio;
    609                 }
    610         }
    611 
    612         addr_index = READ_ONCE(alist->preferred);
    613         if (!test_bit(addr_index, &set))
    614                 addr_index = __ffs(set);
    615 
    616         op->addr_index = addr_index;
    617         set_bit(addr_index, &op->addr_tried);
    618 
    619         op->volsync.creation = TIME64_MIN;
    620         op->volsync.update = TIME64_MIN;
    621         op->call_responded = false;
    622         _debug("address [%u] %u/%u %pISp",
    623                op->server_index, addr_index, alist->nr_addrs,
    624                rxrpc_kernel_remote_addr(alist->addrs[op->addr_index].peer));
    625         _leave(" = t");
    626         return true;
    627 
    628 wait_for_more_probe_results:
    629         error = afs_wait_for_one_fs_probe(op->server, op->estate, op->addr_tried,
    630                                           !(op->flags & AFS_OPERATION_UNINTR));
    631         if (!error)
    632                 goto iterate_address;
    633 
    634         /* We've now had a failure to respond on all of a server's addresses -
    635          * immediately probe them again and consider retrying the server.
    636          */
    637         trace_afs_rotate(op, afs_rotate_trace_probe_fileserver, 0);
    638         afs_probe_fileserver(op->net, op->server);
    639         if (op->flags & AFS_OPERATION_RETRY_SERVER) {
    640                 error = afs_wait_for_one_fs_probe(op->server, op->estate, op->addr_tried,
    641                                                   !(op->flags & AFS_OPERATION_UNINTR));
    642                 switch (error) {
    643                 case 0:
    644                         op->flags &= ~AFS_OPERATION_RETRY_SERVER;
    645                         trace_afs_rotate(op, afs_rotate_trace_retry_server, 0);
    646                         goto retry_server;
    647                 case -ERESTARTSYS:
    648                         afs_op_set_error(op, error);
    649                         goto failed;
    650                 case -ETIME:
    651                 case -EDESTADDRREQ:
    652                         goto next_server;
    653                 }
    654         }
    655 
    656 next_server:
    657         trace_afs_rotate(op, afs_rotate_trace_next_server, 0);
    658         _debug("next");
    659         ASSERT(op->estate);
    660         alist = op->estate->addresses;
    661         if (op->call_responded &&
    662             op->addr_index != READ_ONCE(alist->preferred) &&
    663             test_bit(alist->preferred, &op->addr_tried))
--> 664                 WRITE_ONCE(alist->preferred, op->addr_index);

But then it becomes 0xff here in alist->preferred.

    665         op->estate = NULL;
    666         goto pick_server;
    667 
    668 no_more_servers:
    669         /* That's all the servers poked to no good effect.  Try again if some
    670          * of them were busy.
    671          */
    672         trace_afs_rotate(op, afs_rotate_trace_no_more_servers, 0);
    673         if (op->flags & AFS_OPERATION_VBUSY) {
    674                 afs_sleep_and_retry(op);
    675                 op->flags &= ~AFS_OPERATION_VBUSY;
    676                 goto restart_from_beginning;
    677         }
    678 
    679         rcu_read_lock();
    680         for (i = 0; i < op->server_list->nr_servers; i++) {
    681                 struct afs_endpoint_state *estate;
    682 
    683                 estate = op->server_states->endpoint_state;
    684                 error = READ_ONCE(estate->error);
    685                 if (error < 0)
    686                         afs_op_accumulate_error(op, error, estate->abort_code);
    687         }
    688         rcu_read_unlock();
    689 
    690 failed:
    691         trace_afs_rotate(op, afs_rotate_trace_failed, 0);
    692         op->flags |= AFS_OPERATION_STOP;
    693         if (op->estate) {
    694                 alist = op->estate->addresses;
    695                 if (op->call_responded &&
    696                     op->addr_index != READ_ONCE(alist->preferred) &&

I don't know if it causes an issue, but here -1 and 0xff can't be the
same.

    697                     test_bit(alist->preferred, &op->addr_tried))
    698                         WRITE_ONCE(alist->preferred, op->addr_index);
    699                 op->estate = NULL;
    700         }
    701         _leave(" = f [failed %d]", afs_op_error(op));
    702         return false;
    703 }

regards,
dan carpenter