Solr
  1. Solr
  2. SOLR-6457

LBHttpSolrServer: AIOOBE risk if counter overflows

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0, 4.1, 4.2, 4.2.1, 4.3, 4.3.1, 4.4, 4.5, 4.5.1, 4.6, 4.6.1, 4.7, 4.7.1, 4.7.2, 4.8, 4.8.1, 4.9
    • Fix Version/s: 4.10.4, 5.0, 6.0
    • Component/s: clients - java
    • Labels:

      Description

      org.apache.solr.client.solrj.impl.LBHttpSolrServer
      line 442
      int count = counter.incrementAndGet();
      ServerWrapper wrapper = serverList[count % serverList.length];

      when counter overflows, the mod operation of
      "count % serverList.length" will start trying to use negative numbers as array indexes.

      suggess to fixup it ,eg:
      //keep count is greater than 0
      int count = counter.incrementAndGet() & 0x7FFFFFF;

      1. SOLR-6457.patch
        0.8 kB
        Noble Paul

        Activity

        Hide
        Hoss Man added a comment -

        clarified summary & description to better explain hte problem (counter is an AtomicInteger, so the count can never be greater then Integer.MAX_VALUE – the problem is what happens when it overflows to negative numbers.

        additional questions i have when looking at this code:

        • why is a counter being used here instead of picking a random element?
        • why is the counter an int instead of a long (do we really care about saving a few bytes in this?)
        Show
        Hoss Man added a comment - clarified summary & description to better explain hte problem (counter is an AtomicInteger, so the count can never be greater then Integer.MAX_VALUE – the problem is what happens when it overflows to negative numbers. additional questions i have when looking at this code: why is a counter being used here instead of picking a random element? why is the counter an int instead of a long (do we really care about saving a few bytes in this?)
        Hide
        Noble Paul added a comment -

        why is a counter being used here instead of picking a random element?

        we wanted a more uniform load balancing so , counter helped

        why is the counter an int instead of a long (do we really care about saving a few bytes in this?)

        we don't care , while writing it I didn't think of it

        Show
        Noble Paul added a comment - why is a counter being used here instead of picking a random element? we wanted a more uniform load balancing so , counter helped why is the counter an int instead of a long (do we really care about saving a few bytes in this?) we don't care , while writing it I didn't think of it
        Hide
        ASF subversion and git services added a comment -

        Commit 1622817 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1622817 ]

        SOLR-6457

        Show
        ASF subversion and git services added a comment - Commit 1622817 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1622817 ] SOLR-6457
        Hide
        ASF subversion and git services added a comment -

        Commit 1622818 from Noble Paul in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1622818 ]

        SOLR-6457

        Show
        ASF subversion and git services added a comment - Commit 1622818 from Noble Paul in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1622818 ] SOLR-6457
        Hide
        Noble Paul added a comment -

        thanks longkeyy

        Show
        Noble Paul added a comment - thanks longkeyy
        Hide
        ASF subversion and git services added a comment -

        Commit 1622820 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1622820 ]

        SOLR-6457

        Show
        ASF subversion and git services added a comment - Commit 1622820 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1622820 ] SOLR-6457
        Hide
        ASF subversion and git services added a comment -

        Commit 1622821 from Noble Paul in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1622821 ]

        SOLR-6457

        Show
        ASF subversion and git services added a comment - Commit 1622821 from Noble Paul in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1622821 ] SOLR-6457
        Hide
        Hoss Man added a comment -

        This needs a CHANGES.txt noting the bug fix and giving longkeyy credit for the contribution.

        Show
        Hoss Man added a comment - This needs a CHANGES.txt noting the bug fix and giving longkeyy credit for the contribution.
        Hide
        ASF subversion and git services added a comment -

        Commit 1623744 from Noble Paul in branch 'dev/trunk'
        [ https://svn.apache.org/r1623744 ]

        SOLR-6457

        Show
        ASF subversion and git services added a comment - Commit 1623744 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1623744 ] SOLR-6457
        Hide
        ASF subversion and git services added a comment -

        Commit 1623752 from Noble Paul in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1623752 ]

        SOLR-6457

        Show
        ASF subversion and git services added a comment - Commit 1623752 from Noble Paul in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1623752 ] SOLR-6457
        Hide
        Shalin Shekhar Mangar added a comment -

        Backported to 4.10.4

        Show
        Shalin Shekhar Mangar added a comment - Backported to 4.10.4
        Hide
        ASF subversion and git services added a comment -

        Commit 1662427 from shalin@apache.org in branch 'dev/branches/lucene_solr_4_10'
        [ https://svn.apache.org/r1662427 ]

        SOLR-6457: LBHttpSolrClient: ArrayIndexOutOfBoundsException risk if counter overflows

        Show
        ASF subversion and git services added a comment - Commit 1662427 from shalin@apache.org in branch 'dev/branches/lucene_solr_4_10' [ https://svn.apache.org/r1662427 ] SOLR-6457 : LBHttpSolrClient: ArrayIndexOutOfBoundsException risk if counter overflows
        Hide
        Michael McCandless added a comment -

        Bulk close for 4.10.4 release

        Show
        Michael McCandless added a comment - Bulk close for 4.10.4 release

          People

          • Assignee:
            Noble Paul
            Reporter:
            longkeyy
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development