| 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: <CADvbK_e0F49oNw=erZLCnkYLYP2fvYy92gih0nFpM19JoL=1tQ@mail.gmail.com>
Date: Tue, 10 Sep 2019 14:26:27 +0800
From: Xin Long <lucien.xin@...il.com>
To: Nathan Chancellor <natechancellor@...il.com>,
linux-sctp@...r.kernel.org, network dev <netdev@...r.kernel.org>
Cc: kbuild@...org, Nick Desaulniers <ndesaulniers@...gle.com>,
clang-built-linux@...glegroups.com,
kbuild test robot <lkp@...el.com>
Subject: Re: [PATCH net-next 2/5] sctp: add pf_expose per netns and sock and asoc
On Tue, Sep 10, 2019 at 11:54 AM Nathan Chancellor
<natechancellor@...il.com> wrote:
>
> Hi Xin,
>
> The 0day team has been doing clang builds for us and this warning popped
> up. Let me know if you have any questions.
>
> On Mon, Sep 09, 2019 at 06:44:47PM +0800, kbuild test robot wrote:
> > CC: kbuild-all@...org
> > In-Reply-To: <00fb06e74d8eedeb033dad83de18380bf6261231.1568015756.git.lucien.xin@...il.com>
> > References: <00fb06e74d8eedeb033dad83de18380bf6261231.1568015756.git.lucien.xin@...il.com>
> > TO: Xin Long <lucien.xin@...il.com>
> > CC: network dev <netdev@...r.kernel.org>, linux-sctp@...r.kernel.org
> > CC: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>, Neil Horman <nhorman@...driver.com>, davem@...emloft.net
> >
> > Hi Xin,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on net-next/master]
> >
> > url: https://github.com/0day-ci/linux/commits/Xin-Long/sctp-update-from-rfc7829/20190909-160115
> > config: x86_64-rhel-7.6 (attached as .config)
> > compiler: clang version 10.0.0 (git://gitmirror/llvm_project 45a3fd206fb06f77a08968c99a8172cbf2ccdd0f)
> > reproduce:
> > # save the attached .config to linux build tree
> > make ARCH=x86_64
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@...el.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > >> net/sctp/associola.c:799:24: warning: use of logical '&&' with constant operand [-Wconstant-logical-operand]
> > if (transport->state && SCTP_UNCONFIRMED &&
> > ^ ~~~~~~~~~~~~~~~~
> > net/sctp/associola.c:799:24: note: use '&' for a bitwise operation
> > if (transport->state && SCTP_UNCONFIRMED &&
> > ^~
> > &
> > net/sctp/associola.c:799:24: note: remove constant to silence this warning
> > if (transport->state && SCTP_UNCONFIRMED &&
> > ~^~~~~~~~~~~~~~~~~~~
> > 1 warning generated.
> >
> > vim +799 net/sctp/associola.c
> >
> > 775
> > 776 /* Engage in transport control operations.
> > 777 * Mark the transport up or down and send a notification to the user.
> > 778 * Select and update the new active and retran paths.
> > 779 */
> > 780 void sctp_assoc_control_transport(struct sctp_association *asoc,
> > 781 struct sctp_transport *transport,
> > 782 enum sctp_transport_cmd command,
> > 783 sctp_sn_error_t error)
> > 784 {
> > 785 struct sctp_ulpevent *event;
> > 786 struct sockaddr_storage addr;
> > 787 int spc_state = 0;
> > 788 bool ulp_notify = true;
> > 789
> > 790 /* Record the transition on the transport. */
> > 791 switch (command) {
> > 792 case SCTP_TRANSPORT_UP:
> > 793 /* If we are moving from UNCONFIRMED state due
> > 794 * to heartbeat success, report the SCTP_ADDR_CONFIRMED
> > 795 * state to the user, otherwise report SCTP_ADDR_AVAILABLE.
> > 796 */
> > 797 if (transport->state == SCTP_PF && !asoc->pf_expose)
> > 798 ulp_notify = false;
> > > 799 if (transport->state && SCTP_UNCONFIRMED &&
>
> I assume this && should either be a '&' or '=='?
Right, it should have been "==". It was changed unintentionally
when I swapped the position of 'state' and 'SCTP_UNCONFIRMED'.
Thanks, will post v2 after others' review.
>
> > 800 SCTP_HEARTBEAT_SUCCESS == error)
> > 801 spc_state = SCTP_ADDR_CONFIRMED;
> > 802 else
> > 803 spc_state = SCTP_ADDR_AVAILABLE;
> > 804 transport->state = SCTP_ACTIVE;
> > 805 break;
> > 806
> > 807 case SCTP_TRANSPORT_DOWN:
> > 808 /* If the transport was never confirmed, do not transition it
> > 809 * to inactive state. Also, release the cached route since
> > 810 * there may be a better route next time.
> > 811 */
> > 812 if (transport->state != SCTP_UNCONFIRMED) {
> > 813 transport->state = SCTP_INACTIVE;
> > 814 spc_state = SCTP_ADDR_UNREACHABLE;
> > 815 } else {
> > 816 sctp_transport_dst_release(transport);
> > 817 ulp_notify = false;
> > 818 }
> > 819 break;
> > 820
> > 821 case SCTP_TRANSPORT_PF:
> > 822 transport->state = SCTP_PF;
> > 823 if (!asoc->pf_expose)
> > 824 ulp_notify = false;
> > 825 else
> > 826 spc_state = SCTP_ADDR_POTENTIALLY_FAILED;
> > 827 break;
> > 828
> > 829 default:
> > 830 return;
> > 831 }
> > 832
> > 833 /* Generate and send a SCTP_PEER_ADDR_CHANGE notification
> > 834 * to the user.
> > 835 */
> > 836 if (ulp_notify) {
> > 837 memset(&addr, 0, sizeof(struct sockaddr_storage));
> > 838 memcpy(&addr, &transport->ipaddr,
> > 839 transport->af_specific->sockaddr_len);
> > 840
> > 841 event = sctp_ulpevent_make_peer_addr_change(asoc, &addr,
> > 842 0, spc_state, error, GFP_ATOMIC);
> > 843 if (event)
> > 844 asoc->stream.si->enqueue_event(&asoc->ulpq, event);
> > 845 }
> > 846
> > 847 /* Select new active and retran paths. */
> > 848 sctp_select_active_and_retran_path(asoc);
> > 849 }
> > 850
> >
> > ---
> > 0-DAY kernel test infrastructure Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
> Cheers,
> Nathan
Powered by blists - more mailing lists