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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
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